Upstream patches for Samsung Exynos s5p-mfc and gsc-m2m

Upstream patches for Samsung Exynos s5p-mfc and gsc-m2m

[John Sheu]

This patchset upstreams some changes carried in the ChromeOS kernel tree
especially regarding the s5p-mfc video encoder and gsc-m2m color converter
hardware functionality.

Patch 4 in particular affects the V4L2 interface by allowing VIDIOC_REQBUFS(0)
to succeed when the queue is of type V4L2_MEMORY_MMAP, inline with the rest
of the queue memory types.

[PATCH 1/4] s5p-mfc: fix encoder crash after VIDIOC_STREAMOFF

[John Sheu]

VIDIOC_STREAMOFF clears the encoder's destination queue -- routines run
from the interrupt handler cannot assume that the queue is non-empty.

Signed-off-by: John Sheu <sheu@google.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index df83cd15..04236229 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -772,13 +772,16 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx)
 
 	if (p->seq_hdr_mode == V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE) {
 		spin_lock_irqsave(&dev->irqlock, flags);
-		dst_mb = list_entry(ctx->dst_queue.next,
-				struct s5p_mfc_buf, list);
-		list_del(&dst_mb->list);
-		ctx->dst_queue_cnt--;
-		vb2_set_plane_payload(dst_mb->b, 0,
-			s5p_mfc_hw_call(dev->mfc_ops, get_enc_strm_size, dev));
-		vb2_buffer_done(dst_mb->b, VB2_BUF_STATE_DONE);
+		if (!list_empty(&ctx->dst_queue)) {
+			dst_mb = list_entry(ctx->dst_queue.next,
+					struct s5p_mfc_buf, list);
+			list_del(&dst_mb->list);
+			ctx->dst_queue_cnt--;
+			vb2_set_plane_payload(dst_mb->b, 0,
+				s5p_mfc_hw_call(dev->mfc_ops, get_enc_strm_size,
+						dev));
+			vb2_buffer_done(dst_mb->b, VB2_BUF_STATE_DONE);
+		}
 		spin_unlock_irqrestore(&dev->irqlock, flags);
 	}
 
@@ -883,8 +886,7 @@ static int enc_post_frame_start(struct s5p_mfc_ctx *ctx)
 		mfc_debug(2, "enc src count: %d, enc ref count: %d\n",
 			  ctx->src_queue_cnt, ctx->ref_queue_cnt);
 	}
-	if (strm_size > 0) {
-		/* at least one more dest. buffers exist always  */
+	if ((ctx->dst_queue_cnt > 0) && (strm_size > 0)) {
 		mb_entry = list_entry(ctx->dst_queue.next, struct s5p_mfc_buf,
 									list);
 		list_del(&mb_entry->list);
-- 
1.9.0.279.gdc9e3eb

[PATCH 2/4] CHROMIUM: s5p-mfc: support dynamic encoding parameter changes

[John Sheu]

Add support for dynamic encoding parameter changes in MFCv6.  Parameters
set are applied with the next OUTPUT buffer queued to the device with
VIDIOC_QBUF.

Supported parameters are:

* GOP size (V4L2_CID_MPEG_VIDEO_GOP_SIZE)
* framerate (from VIDIOC_S_PARM)
* VBR target bitrate (V4L2_CID_MPEG_VIDEO_BITRATE)
* (h264) frame type (V4L2_CID_MPEG_MFC51_VIDEO_FORCE_FRAME_TYPE)

Signed-off-by: John Sheu <sheu@google.com>
---
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h    |  4 ++
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h | 31 ++++++--
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    | 53 +++++++++++---
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c | 29 ++++----
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c | 94 ++++++++++++++-----------
 5 files changed, 140 insertions(+), 71 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
index 8d0b686d..68128073 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
@@ -331,6 +331,10 @@
 #define S5P_FIMV_E_MVC_RC_RPARA_VIEW1_V6		0xfd50
 #define S5P_FIMV_E_MVC_INTER_VIEW_PREDICTION_ON_V6	0xfd80
 
+#define S5P_FIMV_E_GOP_CONFIG_CHANGE_SHIFT_V6		0
+#define S5P_FIMV_E_FRAME_RATE_CHANGE_SHIFT_V6		1
+#define S5P_FIMV_E_BIT_RATE_CHANGE_SHIFT_V6		2
+
 /* Codec numbers  */
 #define S5P_FIMV_CODEC_NONE_V6		-1
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 5c28cc3e..90b66d2b 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -164,12 +164,35 @@ enum s5p_mfc_decode_arg {
 	MFC_DEC_RES_CHANGE,
 };
 
+
+/**
+ * enum s5p_mfc_encoder_param_change - indicates runtime parameter change
+ */
+enum s5p_mfc_encode_param_change {
+	MFC_ENC_GOP_CONFIG_CHANGE,
+	MFC_ENC_FRAME_RATE_CHANGE,
+	MFC_ENC_BIT_RATE_CHANGE,
+	MFC_ENC_FRAME_INSERTION,
+};
+
 #define MFC_BUF_FLAG_USED	(1 << 0)
 #define MFC_BUF_FLAG_EOS	(1 << 1)
 
 struct s5p_mfc_ctx;
 
 /**
+ * struct s5p_mfc_enc_params - runtime modifiable encoding parameters
+ */
+struct s5p_mfc_runtime_enc_params {
+	u32 params_changed;
+	u16 gop_size;
+	u32 rc_framerate_num;
+	u32 rc_framerate_denom;
+	u32 rc_bitrate;
+	enum v4l2_mpeg_mfc51_video_force_frame_type force_frame_type;
+};
+
+/**
  * struct s5p_mfc_buf - MFC buffer
  */
 struct s5p_mfc_buf {
@@ -183,6 +206,7 @@ struct s5p_mfc_buf {
 		size_t stream;
 	} cookie;
 	int flags;
+	struct s5p_mfc_runtime_enc_params runtime_enc_params;
 };
 
 /**
@@ -429,7 +453,6 @@ struct s5p_mfc_enc_params {
 	u32 mv_h_range;
 	u32 mv_v_range;
 
-	u16 gop_size;
 	enum v4l2_mpeg_video_multi_slice_mode slice_mode;
 	u16 slice_mb;
 	u32 slice_bit;
@@ -440,7 +463,6 @@ struct s5p_mfc_enc_params {
 	u8 pad_cr;
 	int rc_frame;
 	int rc_mb;
-	u32 rc_bitrate;
 	u16 rc_reaction_coeff;
 	u16 vbv_size;
 	u32 vbv_delay;
@@ -450,10 +472,9 @@ struct s5p_mfc_enc_params {
 	int fixed_target_bit;
 
 	u8 num_b_frame;
-	u32 rc_framerate_num;
-	u32 rc_framerate_denom;
 
 	struct {
+		struct s5p_mfc_runtime_enc_params runtime;
 		struct s5p_mfc_h264_enc_params h264;
 		struct s5p_mfc_mpeg4_enc_params mpeg4;
 		struct s5p_mfc_vp8_enc_params vp8;
@@ -634,8 +655,6 @@ struct s5p_mfc_ctx {
 	size_t me_buffer_size;
 	size_t tmv_buffer_size;
 
-	enum v4l2_mpeg_mfc51_video_force_frame_type force_frame_type;
-
 	struct list_head ref_queue;
 	unsigned int ref_queue_cnt;
 
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 04236229..465eb08c 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -1400,7 +1400,9 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
 
 	switch (ctrl->id) {
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
-		p->gop_size = ctrl->val;
+		p->codec.runtime.params_changed |=
+			(1 << MFC_ENC_GOP_CONFIG_CHANGE);
+		p->codec.runtime.gop_size = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_MULTI_SLICE_MODE:
 		p->slice_mode = ctrl->val;
@@ -1426,13 +1428,17 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
 		p->rc_frame = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_BITRATE:
-		p->rc_bitrate = ctrl->val;
+		p->codec.runtime.params_changed |=
+			(1 << MFC_ENC_BIT_RATE_CHANGE);
+		p->codec.runtime.rc_bitrate = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_MFC51_VIDEO_RC_REACTION_COEFF:
 		p->rc_reaction_coeff = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_MFC51_VIDEO_FORCE_FRAME_TYPE:
-		ctx->force_frame_type = ctrl->val;
+		p->codec.runtime.params_changed |=
+			(1 << MFC_ENC_FRAME_INSERTION);
+		p->codec.runtime.force_frame_type = ctrl->val;
 		break;
 	case V4L2_CID_MPEG_VIDEO_VBV_SIZE:
 		p->vbv_size = ctrl->val;
@@ -1654,12 +1660,29 @@ static int vidioc_s_parm(struct file *file, void *priv,
 			 struct v4l2_streamparm *a)
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+	struct s5p_mfc_runtime_enc_params *runtime_params =
+		&ctx->enc_params.codec.runtime;
 
+	/*
+	 * Note that MFC hardware specifies framerate but the V4L2 API specifies
+	 * timeperframe. Take the reciprocal of one to get the other.
+	 */
 	if (a->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
-		ctx->enc_params.rc_framerate_num =
-					a->parm.output.timeperframe.denominator;
-		ctx->enc_params.rc_framerate_denom =
-					a->parm.output.timeperframe.numerator;
+		if ((runtime_params->rc_framerate_num !=
+			a->parm.output.timeperframe.denominator) ||
+			(runtime_params->rc_framerate_denom !=
+			a->parm.output.timeperframe.numerator)) {
+			if (a->parm.output.timeperframe.numerator == 0) {
+				mfc_err("Cannot set an infinite FPS\n");
+				return -EINVAL;
+			}
+			runtime_params->params_changed |=
+				(1 << MFC_ENC_FRAME_RATE_CHANGE);
+			runtime_params->rc_framerate_num =
+				a->parm.output.timeperframe.denominator;
+			runtime_params->rc_framerate_denom =
+				a->parm.output.timeperframe.numerator;
+		}
 	} else {
 		mfc_err("Setting FPS is only possible for the output queue\n");
 		return -EINVAL;
@@ -1674,9 +1697,9 @@ static int vidioc_g_parm(struct file *file, void *priv,
 
 	if (a->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
 		a->parm.output.timeperframe.denominator =
-					ctx->enc_params.rc_framerate_num;
+			ctx->enc_params.codec.runtime.rc_framerate_num;
 		a->parm.output.timeperframe.numerator =
-					ctx->enc_params.rc_framerate_denom;
+			ctx->enc_params.codec.runtime.rc_framerate_denom;
 	} else {
 		mfc_err("Setting FPS is only possible for the output queue\n");
 		return -EINVAL;
@@ -2010,7 +2033,19 @@ static void s5p_mfc_buf_queue(struct vb2_buffer *vb)
 		ctx->dst_queue_cnt++;
 		spin_unlock_irqrestore(&dev->irqlock, flags);
 	} else if (vq->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		struct s5p_mfc_runtime_enc_params *runtime_params =
+			&ctx->enc_params.codec.runtime;
 		mfc_buf = &ctx->src_bufs[vb->v4l2_buf.index];
+		/* Tag buffer with runtime encoding parameters */
+		memcpy(&mfc_buf->runtime_enc_params, runtime_params,
+			sizeof(mfc_buf->runtime_enc_params));
+		runtime_params->params_changed = 0;
+		/* force_frame_type needs to revert to 0 after being sent. */
+		if (runtime_params->force_frame_type != 0) {
+			v4l2_ctrl_s_ctrl(v4l2_ctrl_find(&ctx->ctrl_handler,
+				V4L2_CID_MPEG_MFC51_VIDEO_FORCE_FRAME_TYPE),
+				V4L2_MPEG_MFC51_VIDEO_FORCE_FRAME_TYPE_DISABLED);
+		}
 		mfc_buf->flags &= ~MFC_BUF_FLAG_USED;
 		spin_lock_irqsave(&dev->irqlock, flags);
 		list_add_tail(&mfc_buf->list, &ctx->src_queue);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
index 58ec7bb2..45646729 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v5.c
@@ -688,7 +688,7 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 	reg = mfc_read(dev, S5P_FIMV_ENC_PIC_TYPE_CTRL);
 	reg |= (1 << 18);
 	reg &= ~(0xFFFF);
-	reg |= p->gop_size;
+	reg |= p->codec.runtime.gop_size;
 	mfc_write(dev, reg, S5P_FIMV_ENC_PIC_TYPE_CTRL);
 	mfc_write(dev, 0, S5P_FIMV_ENC_B_RECON_WRITE_ON);
 	/* multi-slice control */
@@ -736,7 +736,7 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 	mfc_write(dev, reg, S5P_FIMV_ENC_RC_CONFIG);
 	/* bit rate */
 	if (p->rc_frame)
-		mfc_write(dev, p->rc_bitrate,
+		mfc_write(dev, p->codec.runtime.rc_bitrate,
 			S5P_FIMV_ENC_RC_BIT_RATE);
 	else
 		mfc_write(dev, 0, S5P_FIMV_ENC_RC_BIT_RATE);
@@ -831,9 +831,10 @@ static int s5p_mfc_set_enc_params_h264(struct s5p_mfc_ctx *ctx)
 	reg |= p_264->rc_frame_qp;
 	mfc_write(dev, reg, S5P_FIMV_ENC_RC_CONFIG);
 	/* frame rate */
-	if (p->rc_frame && p->rc_framerate_denom)
-		mfc_write(dev, p->rc_framerate_num * 1000
-			/ p->rc_framerate_denom, S5P_FIMV_ENC_RC_FRAME_RATE);
+	if (p->rc_frame && p->codec.runtime.rc_framerate_denom)
+		mfc_write(dev, p->codec.runtime.rc_framerate_num * 1000
+			/ p->codec.runtime.rc_framerate_denom,
+			S5P_FIMV_ENC_RC_FRAME_RATE);
 	else
 		mfc_write(dev, 0, S5P_FIMV_ENC_RC_FRAME_RATE);
 	/* max & min value of QP */
@@ -950,16 +951,17 @@ static int s5p_mfc_set_enc_params_mpeg4(struct s5p_mfc_ctx *ctx)
 	}
 	/* frame rate */
 	if (p->rc_frame) {
-		if (p->rc_framerate_denom > 0) {
-			framerate = p->rc_framerate_num * 1000 /
-						p->rc_framerate_denom;
+		if (p->codec.runtime.rc_framerate_denom > 0) {
+			framerate = p->codec.runtime.rc_framerate_num * 1000 /
+				p->codec.runtime.rc_framerate_denom;
 			mfc_write(dev, framerate,
 				S5P_FIMV_ENC_RC_FRAME_RATE);
 			shm = s5p_mfc_read_info_v5(ctx, RC_VOP_TIMING);
 			shm &= ~(0xFFFFFFFF);
 			shm |= (1 << 31);
-			shm |= ((p->rc_framerate_num & 0x7FFF) << 16);
-			shm |= (p->rc_framerate_denom & 0xFFFF);
+			shm |= ((p->codec.runtime.rc_framerate_num & 0x7FFF)
+				<< 16);
+			shm |= (p->codec.runtime.rc_framerate_denom & 0xFFFF);
 			s5p_mfc_write_info_v5(ctx, shm, RC_VOP_TIMING);
 		}
 	} else {
@@ -1009,9 +1011,10 @@ static int s5p_mfc_set_enc_params_h263(struct s5p_mfc_ctx *ctx)
 		s5p_mfc_write_info_v5(ctx, shm, P_B_FRAME_QP);
 	}
 	/* frame rate */
-	if (p->rc_frame && p->rc_framerate_denom)
-		mfc_write(dev, p->rc_framerate_num * 1000
-			/ p->rc_framerate_denom, S5P_FIMV_ENC_RC_FRAME_RATE);
+	if (p->rc_frame && p->codec.runtime.rc_framerate_denom)
+		mfc_write(dev, p->codec.runtime.rc_framerate_num * 1000
+		/ p->codec.runtime.rc_framerate_denom,
+		S5P_FIMV_ENC_RC_FRAME_RATE);
 	else
 		mfc_write(dev, 0, S5P_FIMV_ENC_RC_FRAME_RATE);
 	/* rate control config. */
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 f64621ae..89c6dffe 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -587,6 +587,52 @@ static int s5p_mfc_set_slice_mode(struct s5p_mfc_ctx *ctx)
 	return 0;
 }
 
+static int s5p_mfc_set_runtime_enc_params(struct s5p_mfc_ctx *ctx,
+		struct s5p_mfc_runtime_enc_params *runtime_p)
+{
+	struct s5p_mfc_dev *dev = ctx->dev;
+	struct s5p_mfc_enc_params *p = &ctx->enc_params;
+	unsigned int params_changed = 0;
+
+	if (runtime_p->params_changed & (1 << MFC_ENC_GOP_CONFIG_CHANGE)) {
+		params_changed |= (1 << S5P_FIMV_E_GOP_CONFIG_CHANGE_SHIFT_V6);
+		/* pictype: IDR period */
+		WRITEL((runtime_p->gop_size & 0xFFFF),
+			S5P_FIMV_E_GOP_CONFIG_V6);
+	}
+	if (runtime_p->params_changed & (1 << MFC_ENC_FRAME_RATE_CHANGE)) {
+		/* frame rate */
+		if (p->rc_frame && runtime_p->rc_framerate_num &&
+			runtime_p->rc_framerate_denom) {
+			params_changed |=
+				(1 << S5P_FIMV_E_FRAME_RATE_CHANGE_SHIFT_V6);
+			WRITEL((((runtime_p->rc_framerate_num & 0xFFFF) << 16) |
+				(runtime_p->rc_framerate_denom & 0xFFFF)),
+				S5P_FIMV_E_RC_FRAME_RATE_V6);
+		}
+	}
+	if (runtime_p->params_changed & (1 << MFC_ENC_BIT_RATE_CHANGE)) {
+		/* bit rate */
+		if (p->rc_frame) {
+			params_changed |=
+				(1 << S5P_FIMV_E_BIT_RATE_CHANGE_SHIFT_V6);
+			WRITEL(runtime_p->rc_bitrate,
+				S5P_FIMV_E_RC_BIT_RATE_V6);
+		}
+	}
+	if (runtime_p->params_changed & (1 << MFC_ENC_FRAME_INSERTION)) {
+		unsigned int reg = READL(S5P_FIMV_E_FRAME_INSERTION_V6);
+		reg &= ~0x3;
+		reg |= runtime_p->force_frame_type & 0x3;
+		WRITEL(reg, S5P_FIMV_E_FRAME_INSERTION_V6);
+	}
+
+	if (params_changed)
+		WRITEL(params_changed, S5P_FIMV_E_PARAM_CHANGE_V6);
+
+	return 0;
+}
+
 static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 {
 	struct s5p_mfc_dev *dev = ctx->dev;
@@ -607,10 +653,10 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 	/* cropped offset */
 	WRITEL(0x0, S5P_FIMV_E_FRAME_CROP_OFFSET_V6);
 
-	/* pictype : IDR period */
-	reg = 0;
-	reg |= p->gop_size & 0xFFFF;
-	WRITEL(reg, S5P_FIMV_E_GOP_CONFIG_V6);
+	/* send all runtime encoder parameters. */
+	p->codec.runtime.params_changed = ~0;
+	s5p_mfc_set_runtime_enc_params(ctx, &p->codec.runtime);
+	p->codec.runtime.params_changed = 0;
 
 	/* multi-slice control */
 	/* multi-slice MB number or bit size */
@@ -696,13 +742,6 @@ static int s5p_mfc_set_enc_params(struct s5p_mfc_ctx *ctx)
 	reg |= ((p->rc_frame & 0x1) << 9);
 	WRITEL(reg, S5P_FIMV_E_RC_CONFIG_V6);
 
-	/* bit rate */
-	if (p->rc_frame)
-		WRITEL(p->rc_bitrate,
-			S5P_FIMV_E_RC_BIT_RATE_V6);
-	else
-		WRITEL(1, S5P_FIMV_E_RC_BIT_RATE_V6);
-
 	/* reaction coefficient */
 	if (p->rc_frame) {
 		if (p->rc_reaction_coeff < TIGHT_CBR_MAX) /* tight CBR */
@@ -806,14 +845,6 @@ static int s5p_mfc_set_enc_params_h264(struct s5p_mfc_ctx *ctx)
 		WRITEL(reg, S5P_FIMV_E_FIXED_PICTURE_QP_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) {
@@ -1081,14 +1112,6 @@ static int s5p_mfc_set_enc_params_mpeg4(struct s5p_mfc_ctx *ctx)
 		WRITEL(reg, S5P_FIMV_E_FIXED_PICTURE_QP_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) {
@@ -1153,14 +1176,6 @@ static int s5p_mfc_set_enc_params_h263(struct s5p_mfc_ctx *ctx)
 		WRITEL(reg, S5P_FIMV_E_FIXED_PICTURE_QP_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) {
@@ -1204,14 +1219,6 @@ static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx)
 	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);
-	}
-
 	/* frame QP */
 	reg &= ~(0x7F);
 	reg |= p_vp8->rc_frame_qp & 0x7F;
@@ -1573,6 +1580,7 @@ static inline int s5p_mfc_run_enc_frame(struct s5p_mfc_ctx *ctx)
 	mfc_debug(2, "enc src c addr: 0x%08lx\n", src_c_addr);
 
 	s5p_mfc_set_enc_frame_buffer_v6(ctx, src_y_addr, src_c_addr);
+	s5p_mfc_set_runtime_enc_params(ctx, &src_mb->runtime_enc_params);
 
 	dst_mb = list_entry(ctx->dst_queue.next, struct s5p_mfc_buf, list);
 	dst_mb->flags |= MFC_BUF_FLAG_USED;
-- 
1.9.0.279.gdc9e3eb

[PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage

[John Sheu]

Explicitly specify sampling period for subsampled chroma formats, so
stride and image size are properly reported through VIDIOC_{S,G}_FMT.

Signed-off-by: John Sheu <sheu@google.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.c | 154 +++++++++++++++------------
 drivers/media/platform/exynos-gsc/gsc-core.h |  16 +--
 drivers/media/platform/exynos-gsc/gsc-regs.c |  40 +++----
 drivers/media/platform/exynos-gsc/gsc-regs.h |   4 +-
 4 files changed, 116 insertions(+), 98 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
index 9d0cc04d..c02addef 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.c
+++ b/drivers/media/platform/exynos-gsc/gsc-core.c
@@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
 	{
 		.name		= "RGB565",
 		.pixelformat	= V4L2_PIX_FMT_RGB565X,
-		.depth		= { 16 },
 		.color		= GSC_RGB,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 16 },
+		.sampling	= { { 1, 1 } },
 	}, {
 		.name		= "XRGB-8-8-8-8, 32 bpp",
 		.pixelformat	= V4L2_PIX_FMT_RGB32,
-		.depth		= { 32 },
 		.color		= GSC_RGB,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 32 },
+		.sampling	= { { 1, 1 } },
 	}, {
 		.name		= "YUV 4:2:2 packed, YCbYCr",
 		.pixelformat	= V4L2_PIX_FMT_YUYV,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 16 },
+		.sampling	= { { 1, 1 } },
 		.mbus_code	= V4L2_MBUS_FMT_YUYV8_2X8,
 	}, {
 		.name		= "YUV 4:2:2 packed, CbYCrY",
 		.pixelformat	= V4L2_PIX_FMT_UYVY,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_C,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 16 },
+		.sampling	= { { 1, 1 } },
 		.mbus_code	= V4L2_MBUS_FMT_UYVY8_2X8,
 	}, {
 		.name		= "YUV 4:2:2 packed, CrYCbY",
 		.pixelformat	= V4L2_PIX_FMT_VYUY,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_C,
 		.corder		= GSC_CRCB,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 16 },
+		.sampling	= { { 1, 1 } },
 		.mbus_code	= V4L2_MBUS_FMT_VYUY8_2X8,
 	}, {
 		.name		= "YUV 4:2:2 packed, YCrYCb",
 		.pixelformat	= V4L2_PIX_FMT_YVYU,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CRCB,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 16 },
+		.sampling	= { { 1, 1 } },
 		.mbus_code	= V4L2_MBUS_FMT_YVYU8_2X8,
 	}, {
 		.name		= "YUV 4:4:4 planar, YCbYCr",
 		.pixelformat	= V4L2_PIX_FMT_YUV32,
-		.depth		= { 32 },
 		.color		= GSC_YUV444,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 1,
+		.depth		= { 32 },
+		.sampling	= { { 1, 1 } },
 	}, {
 		.name		= "YUV 4:2:2 planar, Y/Cb/Cr",
 		.pixelformat	= V4L2_PIX_FMT_YUV422P,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 3,
+		.depth		= { 8, 8, 8 },
+		.sampling	= { { 1, 1 }, { 2, 1 }, { 2, 1 } },
 	}, {
 		.name		= "YUV 4:2:2 planar, Y/CbCr",
 		.pixelformat	= V4L2_PIX_FMT_NV16,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 2,
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 1 } },
 	}, {
 		.name		= "YUV 4:2:2 planar, Y/CrCb",
 		.pixelformat	= V4L2_PIX_FMT_NV61,
-		.depth		= { 16 },
 		.color		= GSC_YUV422,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CRCB,
 		.num_planes	= 1,
 		.num_comp	= 2,
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 1 } },
 	}, {
 		.name		= "YUV 4:2:0 planar, YCbCr",
 		.pixelformat	= V4L2_PIX_FMT_YUV420,
-		.depth		= { 12 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 3,
+		.depth		= { 8, 8, 8 },
+		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
 	}, {
 		.name		= "YUV 4:2:0 planar, YCrCb",
 		.pixelformat	= V4L2_PIX_FMT_YVU420,
-		.depth		= { 12 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CRCB,
 		.num_planes	= 1,
 		.num_comp	= 3,
-
+		.depth		= { 8, 8, 8 },
+		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
 	}, {
 		.name		= "YUV 4:2:0 planar, Y/CbCr",
 		.pixelformat	= V4L2_PIX_FMT_NV12,
-		.depth		= { 12 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 1,
 		.num_comp	= 2,
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 2 } },
 	}, {
 		.name		= "YUV 4:2:0 planar, Y/CrCb",
 		.pixelformat	= V4L2_PIX_FMT_NV21,
-		.depth		= { 12 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CRCB,
 		.num_planes	= 1,
 		.num_comp	= 2,
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 2 } },
 	}, {
-		.name		= "YUV 4:2:0 non-contig. 2p, Y/CbCr",
+		.name		= "YUV 4:2:0 non-contiguous 2-planar, Y/CbCr",
 		.pixelformat	= V4L2_PIX_FMT_NV12M,
-		.depth		= { 8, 4 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 2,
 		.num_comp	= 2,
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 2 } },
 	}, {
-		.name		= "YUV 4:2:0 non-contig. 3p, Y/Cb/Cr",
+		.name		= "YUV 4:2:0 non-contiguous 3-planar, Y/Cb/Cr",
 		.pixelformat	= V4L2_PIX_FMT_YUV420M,
-		.depth		= { 8, 2, 2 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 3,
 		.num_comp	= 3,
+		.depth		= { 8, 8, 8 },
+		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
 	}, {
-		.name		= "YUV 4:2:0 non-contig. 3p, Y/Cr/Cb",
+		.name		= "YUV 4:2:0 non-contiguous 3-planar, Y/Cr/Cb",
 		.pixelformat	= V4L2_PIX_FMT_YVU420M,
-		.depth		= { 8, 2, 2 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CRCB,
 		.num_planes	= 3,
 		.num_comp	= 3,
+		.depth		= { 8, 8, 8 },
+		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
 	}, {
-		.name		= "YUV 4:2:0 n.c. 2p, Y/CbCr tiled",
+		.name		=
+			"YUV 4:2:0 non-contiguous 2-planar, Y/CbCr, tiled",
 		.pixelformat	= V4L2_PIX_FMT_NV12MT_16X16,
-		.depth		= { 8, 4 },
 		.color		= GSC_YUV420,
 		.yorder		= GSC_LSB_Y,
 		.corder		= GSC_CBCR,
 		.num_planes	= 2,
 		.num_comp	= 2,
-	}
+		.depth		= { 8, 16 },
+		.sampling	= { { 1, 1 }, { 2, 2 } },
+	},
 };
 
 const struct gsc_fmt *get_format(int index)
@@ -384,6 +402,14 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
 			f_chk_addr, f_chk_len, s_chk_addr, s_chk_len);
 }
 
+static void get_format_size(__u32 width, __u32 height,
+			    const struct gsc_fmt *fmt, int plane,
+			    __u16 *bytesperline, __u32 *sizeimage) {
+	__u16 bpl = ((width * fmt->depth[plane]) / fmt->sampling[plane][0]) / 8;
+	*bytesperline = bpl;
+	*sizeimage = (height * bpl) / fmt->sampling[plane][0];
+}
+
 int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
 {
 	struct gsc_dev *gsc = ctx->gsc_dev;
@@ -448,14 +474,16 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
 	else /* SD */
 		pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
 
-
+	/* V4L2 specifies for contiguous planar formats that bytesperline and
+	   sizeimage are set to values appropriate for the first plane. */
 	for (i = 0; i < pix_mp->num_planes; ++i) {
-		int bpl = (pix_mp->width * fmt->depth[i]) >> 3;
-		pix_mp->plane_fmt[i].bytesperline = bpl;
-		pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height;
+		get_format_size(pix_mp->width, pix_mp->height, fmt, i,
+				&pix_mp->plane_fmt[i].bytesperline,
+				&pix_mp->plane_fmt[i].sizeimage);
 
 		pr_debug("[%d]: bpl: %d, sizeimage: %d",
-				i, bpl, pix_mp->plane_fmt[i].sizeimage);
+				i, pix_mp->plane_fmt[i].bytesperline,
+				pix_mp->plane_fmt[i].sizeimage);
 	}
 
 	return 0;
@@ -465,26 +493,27 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
 {
 	struct gsc_frame *frame;
 	struct v4l2_pix_format_mplane *pix_mp;
+	const struct gsc_fmt *fmt;
 	int i;
 
 	frame = ctx_get_frame(ctx, f->type);
 	if (IS_ERR(frame))
 		return PTR_ERR(frame);
+	fmt = frame->fmt;
 
 	pix_mp = &f->fmt.pix_mp;
 
 	pix_mp->width		= frame->f_width;
 	pix_mp->height		= frame->f_height;
 	pix_mp->field		= V4L2_FIELD_NONE;
-	pix_mp->pixelformat	= frame->fmt->pixelformat;
+	pix_mp->pixelformat	= fmt->pixelformat;
 	pix_mp->colorspace	= V4L2_COLORSPACE_REC709;
-	pix_mp->num_planes	= frame->fmt->num_planes;
+	pix_mp->num_planes	= fmt->num_planes;
 
 	for (i = 0; i < pix_mp->num_planes; ++i) {
-		pix_mp->plane_fmt[i].bytesperline = (frame->f_width *
-			frame->fmt->depth[i]) / 8;
-		pix_mp->plane_fmt[i].sizeimage =
-			 pix_mp->plane_fmt[i].bytesperline * frame->f_height;
+		get_format_size(pix_mp->width, pix_mp->height, fmt, i,
+				&pix_mp->plane_fmt[i].bytesperline,
+				&pix_mp->plane_fmt[i].sizeimage);
 	}
 
 	return 0;
@@ -794,11 +823,12 @@ void gsc_ctrls_delete(struct gsc_ctx *ctx)
 	}
 }
 
-/* The color format (num_comp, num_planes) must be already configured. */
+/* The color format (num_planes, num_comp) must be already configured. */
 int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
 			struct gsc_frame *frame, struct gsc_addr *addr)
 {
 	int ret = 0;
+	const struct gsc_fmt *fmt = frame->fmt;
 	u32 pix_size;
 
 	if ((vb == NULL) || (frame == NULL))
@@ -810,46 +840,30 @@ int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
 		frame->fmt->num_planes, frame->fmt->num_comp, pix_size);
 
 	addr->y = vb2_dma_contig_plane_dma_addr(vb, 0);
-
-	if (frame->fmt->num_planes == 1) {
-		switch (frame->fmt->num_comp) {
-		case 1:
-			addr->cb = 0;
-			addr->cr = 0;
-			break;
-		case 2:
-			/* decompose Y into Y/Cb */
-			addr->cb = (dma_addr_t)(addr->y + pix_size);
-			addr->cr = 0;
-			break;
-		case 3:
-			/* decompose Y into Y/Cb/Cr */
-			addr->cb = (dma_addr_t)(addr->y + pix_size);
-			if (GSC_YUV420 == frame->fmt->color)
-				addr->cr = (dma_addr_t)(addr->cb
-						+ (pix_size >> 2));
-			else /* 422 */
-				addr->cr = (dma_addr_t)(addr->cb
-						+ (pix_size >> 1));
-			break;
-		default:
-			pr_err("Invalid the number of color planes");
-			return -EINVAL;
+	addr->cb = 0;
+	addr->cr = 0;
+
+	if (fmt->num_planes == 1) {
+		if (fmt->num_comp >= 2) {
+			addr->cb = (dma_addr_t)(addr->y +
+				((pix_size * fmt->depth[0]) /
+				(fmt->sampling[0][0] *
+				 fmt->sampling[0][1]) / 8));
+		}
+		if (fmt->num_comp >= 3) {
+			addr->cr = (dma_addr_t)(addr->cb +
+				((pix_size * fmt->depth[1]) /
+				(fmt->sampling[1][0] *
+				 fmt->sampling[1][1]) / 8));
 		}
 	} else {
-		if (frame->fmt->num_planes >= 2)
+		if (fmt->num_comp >= 2)
 			addr->cb = vb2_dma_contig_plane_dma_addr(vb, 1);
-
-		if (frame->fmt->num_planes == 3)
+		if (fmt->num_comp == 3)
 			addr->cr = vb2_dma_contig_plane_dma_addr(vb, 2);
 	}
 
-	if ((frame->fmt->pixelformat == V4L2_PIX_FMT_VYUY) ||
-		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVYU) ||
-		(frame->fmt->pixelformat == V4L2_PIX_FMT_NV61) ||
-		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420) ||
-		(frame->fmt->pixelformat == V4L2_PIX_FMT_NV21) ||
-		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420M))
+	if (fmt->corder == GSC_CRCB)
 		swap(addr->cb, addr->cr);
 
 	pr_debug("ADDR: y= 0x%X  cb= 0x%X cr= 0x%X ret= %d",
diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
index ef0a6564..8fb07e0d 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -106,27 +106,27 @@ enum gsc_yuv_fmt {
 	container_of((__ctrl)->handler, struct gsc_ctx, ctrl_handler)
 /**
  * struct gsc_fmt - the driver's internal color format data
- * @mbus_code: Media Bus pixel code, -1 if not applicable
  * @name: format description
  * @pixelformat: the fourcc code for this format, 0 if not applicable
  * @yorder: Y/C order
  * @corder: Chrominance order control
  * @num_planes: number of physically non-contiguous data planes
- * @nr_comp: number of physically contiguous data planes
- * @depth: per plane driver's private 'number of bits per pixel'
- * @flags: flags indicating which operation mode format applies to
+ * @num_comp: number of physically contiguous data planes
+ * @depth: bit depth of each component
+ * @sampling: sampling frequency of each components, X and Y
+ * @mbus_code: Media Bus pixel code, -1 if not applicable
  */
 struct gsc_fmt {
-	enum v4l2_mbus_pixelcode mbus_code;
 	char	*name;
 	u32	pixelformat;
 	u32	color;
 	u32	yorder;
 	u32	corder;
-	u16	num_planes;
-	u16	num_comp;
+	u8	num_planes;
+	u8	num_comp;
 	u8	depth[VIDEO_MAX_PLANES];
-	u32	flags;
+	u8	sampling[VIDEO_MAX_PLANES][2];
+	enum v4l2_mbus_pixelcode mbus_code;
 };
 
 /**
diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.c b/drivers/media/platform/exynos-gsc/gsc-regs.c
index e22d147a..a8d6c90b 100644
--- a/drivers/media/platform/exynos-gsc/gsc-regs.c
+++ b/drivers/media/platform/exynos-gsc/gsc-regs.c
@@ -167,6 +167,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
 {
 	struct gsc_dev *dev = ctx->gsc_dev;
 	struct gsc_frame *frame = &ctx->s_frame;
+	const struct gsc_fmt *fmt = frame->fmt;
 	u32 i, depth = 0;
 	u32 cfg;
 
@@ -176,21 +177,22 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
 		 GSC_IN_TILE_TYPE_MASK | GSC_IN_TILE_MODE);
 	writel(cfg, dev->regs + GSC_IN_CON);
 
-	if (is_rgb(frame->fmt->color)) {
+	if (is_rgb(fmt->color)) {
 		gsc_hw_set_in_image_rgb(ctx);
 		return;
 	}
-	for (i = 0; i < frame->fmt->num_planes; i++)
-		depth += frame->fmt->depth[i];
+	for (i = 0; i < fmt->num_comp; i++)
+		depth += fmt->depth[i] /
+			(fmt->sampling[i][0] * fmt->sampling[i][1]);
 
-	switch (frame->fmt->num_comp) {
+	switch (fmt->num_comp) {
 	case 1:
 		cfg |= GSC_IN_YUV422_1P;
-		if (frame->fmt->yorder == GSC_LSB_Y)
+		if (fmt->yorder == GSC_LSB_Y)
 			cfg |= GSC_IN_YUV422_1P_ORDER_LSB_Y;
 		else
-			cfg |= GSC_IN_YUV422_1P_OEDER_LSB_C;
-		if (frame->fmt->corder == GSC_CBCR)
+			cfg |= GSC_IN_YUV422_1P_ORDER_LSB_C;
+		if (fmt->corder == GSC_CBCR)
 			cfg |= GSC_IN_CHROMA_ORDER_CBCR;
 		else
 			cfg |= GSC_IN_CHROMA_ORDER_CRCB;
@@ -200,7 +202,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
 			cfg |= GSC_IN_YUV420_2P;
 		else
 			cfg |= GSC_IN_YUV422_2P;
-		if (frame->fmt->corder == GSC_CBCR)
+		if (fmt->corder == GSC_CBCR)
 			cfg |= GSC_IN_CHROMA_ORDER_CBCR;
 		else
 			cfg |= GSC_IN_CHROMA_ORDER_CRCB;
@@ -213,7 +215,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
 		break;
 	}
 
-	if (is_tiled(frame->fmt))
+	if (is_tiled(fmt))
 		cfg |= GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE;
 
 	writel(cfg, dev->regs + GSC_IN_CON);
@@ -287,6 +289,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
 {
 	struct gsc_dev *dev = ctx->gsc_dev;
 	struct gsc_frame *frame = &ctx->d_frame;
+	const struct gsc_fmt *fmt = frame->fmt;
 	u32 i, depth = 0;
 	u32 cfg;
 
@@ -296,7 +299,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
 		 GSC_OUT_TILE_TYPE_MASK | GSC_OUT_TILE_MODE);
 	writel(cfg, dev->regs + GSC_OUT_CON);
 
-	if (is_rgb(frame->fmt->color)) {
+	if (is_rgb(fmt->color)) {
 		gsc_hw_set_out_image_rgb(ctx);
 		return;
 	}
@@ -306,17 +309,18 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
 		goto end_set;
 	}
 
-	for (i = 0; i < frame->fmt->num_planes; i++)
-		depth += frame->fmt->depth[i];
+	for (i = 0; i < fmt->num_comp; i++)
+		depth += fmt->depth[i] /
+			(fmt->sampling[i][0] * fmt->sampling[i][1]);
 
-	switch (frame->fmt->num_comp) {
+	switch (fmt->num_comp) {
 	case 1:
 		cfg |= GSC_OUT_YUV422_1P;
-		if (frame->fmt->yorder == GSC_LSB_Y)
+		if (fmt->yorder == GSC_LSB_Y)
 			cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_Y;
 		else
-			cfg |= GSC_OUT_YUV422_1P_OEDER_LSB_C;
-		if (frame->fmt->corder == GSC_CBCR)
+			cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_C;
+		if (fmt->corder == GSC_CBCR)
 			cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
 		else
 			cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
@@ -326,7 +330,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
 			cfg |= GSC_OUT_YUV420_2P;
 		else
 			cfg |= GSC_OUT_YUV422_2P;
-		if (frame->fmt->corder == GSC_CBCR)
+		if (fmt->corder == GSC_CBCR)
 			cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
 		else
 			cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
@@ -336,7 +340,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
 		break;
 	}
 
-	if (is_tiled(frame->fmt))
+	if (is_tiled(fmt))
 		cfg |= GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE;
 
 end_set:
diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.h b/drivers/media/platform/exynos-gsc/gsc-regs.h
index 4678f9a6..b03401dc 100644
--- a/drivers/media/platform/exynos-gsc/gsc-regs.h
+++ b/drivers/media/platform/exynos-gsc/gsc-regs.h
@@ -46,7 +46,7 @@
 #define GSC_IN_RGB_SD_WIDE		(0 << 14)
 #define GSC_IN_YUV422_1P_ORDER_MASK	(1 << 13)
 #define GSC_IN_YUV422_1P_ORDER_LSB_Y	(0 << 13)
-#define GSC_IN_YUV422_1P_OEDER_LSB_C	(1 << 13)
+#define GSC_IN_YUV422_1P_ORDER_LSB_C	(1 << 13)
 #define GSC_IN_CHROMA_ORDER_MASK	(1 << 12)
 #define GSC_IN_CHROMA_ORDER_CBCR	(0 << 12)
 #define GSC_IN_CHROMA_ORDER_CRCB	(1 << 12)
@@ -91,7 +91,7 @@
 #define GSC_OUT_RGB_SD_NARROW		(0 << 10)
 #define GSC_OUT_YUV422_1P_ORDER_MASK	(1 << 9)
 #define GSC_OUT_YUV422_1P_ORDER_LSB_Y	(0 << 9)
-#define GSC_OUT_YUV422_1P_OEDER_LSB_C	(1 << 9)
+#define GSC_OUT_YUV422_1P_ORDER_LSB_C	(1 << 9)
 #define GSC_OUT_CHROMA_ORDER_MASK	(1 << 8)
 #define GSC_OUT_CHROMA_ORDER_CBCR	(0 << 8)
 #define GSC_OUT_CHROMA_ORDER_CRCB	(1 << 8)
-- 
1.9.0.279.gdc9e3eb

RE: [PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage

[Kamil Debski]

Hi Shaik, Sungchun,

Exynos-gsc has no maintainers mentioned in the MAINTAINERS file.
I see your activity in the development of this driver. Do you have
any comments on this patch?

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


> -----Original Message-----
> From: John Sheu [mailto:sheu@google.com]
> Sent: Tuesday, March 11, 2014 11:52 PM
> To: linux-media@vger.kernel.org
> Cc: m.chehab@samsung.com; k.debski@samsung.com; posciak@google.com;
> arun.m@samsung.com; kgene.kim@samsung.com; John Sheu
> Subject: [PATCH 3/4] gsc-m2m: report correct format bytesperline and
> sizeimage
> 
> Explicitly specify sampling period for subsampled chroma formats, so
> stride and image size are properly reported through VIDIOC_{S,G}_FMT.
> 
> Signed-off-by: John Sheu <sheu@google.com>
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.c | 154 +++++++++++++++----
> --------  drivers/media/platform/exynos-gsc/gsc-core.h |  16 +--
> drivers/media/platform/exynos-gsc/gsc-regs.c |  40 +++----
>  drivers/media/platform/exynos-gsc/gsc-regs.h |   4 +-
>  4 files changed, 116 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c
> b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 9d0cc04d..c02addef 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
>  	{
>  		.name		= "RGB565",
>  		.pixelformat	= V4L2_PIX_FMT_RGB565X,
> -		.depth		= { 16 },
>  		.color		= GSC_RGB,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 16 },
> +		.sampling	= { { 1, 1 } },
>  	}, {
>  		.name		= "XRGB-8-8-8-8, 32 bpp",
>  		.pixelformat	= V4L2_PIX_FMT_RGB32,
> -		.depth		= { 32 },
>  		.color		= GSC_RGB,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 32 },
> +		.sampling	= { { 1, 1 } },
>  	}, {
>  		.name		= "YUV 4:2:2 packed, YCbYCr",
>  		.pixelformat	= V4L2_PIX_FMT_YUYV,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 16 },
> +		.sampling	= { { 1, 1 } },
>  		.mbus_code	= V4L2_MBUS_FMT_YUYV8_2X8,
>  	}, {
>  		.name		= "YUV 4:2:2 packed, CbYCrY",
>  		.pixelformat	= V4L2_PIX_FMT_UYVY,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_C,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 16 },
> +		.sampling	= { { 1, 1 } },
>  		.mbus_code	= V4L2_MBUS_FMT_UYVY8_2X8,
>  	}, {
>  		.name		= "YUV 4:2:2 packed, CrYCbY",
>  		.pixelformat	= V4L2_PIX_FMT_VYUY,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_C,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 16 },
> +		.sampling	= { { 1, 1 } },
>  		.mbus_code	= V4L2_MBUS_FMT_VYUY8_2X8,
>  	}, {
>  		.name		= "YUV 4:2:2 packed, YCrYCb",
>  		.pixelformat	= V4L2_PIX_FMT_YVYU,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 16 },
> +		.sampling	= { { 1, 1 } },
>  		.mbus_code	= V4L2_MBUS_FMT_YVYU8_2X8,
>  	}, {
>  		.name		= "YUV 4:4:4 planar, YCbYCr",
>  		.pixelformat	= V4L2_PIX_FMT_YUV32,
> -		.depth		= { 32 },
>  		.color		= GSC_YUV444,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 1,
> +		.depth		= { 32 },
> +		.sampling	= { { 1, 1 } },
>  	}, {
>  		.name		= "YUV 4:2:2 planar, Y/Cb/Cr",
>  		.pixelformat	= V4L2_PIX_FMT_YUV422P,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 3,
> +		.depth		= { 8, 8, 8 },
> +		.sampling	= { { 1, 1 }, { 2, 1 }, { 2, 1 } },
>  	}, {
>  		.name		= "YUV 4:2:2 planar, Y/CbCr",
>  		.pixelformat	= V4L2_PIX_FMT_NV16,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 2,
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 1 } },
>  	}, {
>  		.name		= "YUV 4:2:2 planar, Y/CrCb",
>  		.pixelformat	= V4L2_PIX_FMT_NV61,
> -		.depth		= { 16 },
>  		.color		= GSC_YUV422,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 1,
>  		.num_comp	= 2,
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 1 } },
>  	}, {
>  		.name		= "YUV 4:2:0 planar, YCbCr",
>  		.pixelformat	= V4L2_PIX_FMT_YUV420,
> -		.depth		= { 12 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 3,
> +		.depth		= { 8, 8, 8 },
> +		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>  	}, {
>  		.name		= "YUV 4:2:0 planar, YCrCb",
>  		.pixelformat	= V4L2_PIX_FMT_YVU420,
> -		.depth		= { 12 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 1,
>  		.num_comp	= 3,
> -
> +		.depth		= { 8, 8, 8 },
> +		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>  	}, {
>  		.name		= "YUV 4:2:0 planar, Y/CbCr",
>  		.pixelformat	= V4L2_PIX_FMT_NV12,
> -		.depth		= { 12 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 1,
>  		.num_comp	= 2,
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 2 } },
>  	}, {
>  		.name		= "YUV 4:2:0 planar, Y/CrCb",
>  		.pixelformat	= V4L2_PIX_FMT_NV21,
> -		.depth		= { 12 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 1,
>  		.num_comp	= 2,
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 2 } },
>  	}, {
> -		.name		= "YUV 4:2:0 non-contig. 2p, Y/CbCr",
> +		.name		= "YUV 4:2:0 non-contiguous 2-planar,
Y/CbCr",
>  		.pixelformat	= V4L2_PIX_FMT_NV12M,
> -		.depth		= { 8, 4 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 2,
>  		.num_comp	= 2,
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 2 } },
>  	}, {
> -		.name		= "YUV 4:2:0 non-contig. 3p, Y/Cb/Cr",
> +		.name		= "YUV 4:2:0 non-contiguous 3-planar,
Y/Cb/Cr",
>  		.pixelformat	= V4L2_PIX_FMT_YUV420M,
> -		.depth		= { 8, 2, 2 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 3,
>  		.num_comp	= 3,
> +		.depth		= { 8, 8, 8 },
> +		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>  	}, {
> -		.name		= "YUV 4:2:0 non-contig. 3p, Y/Cr/Cb",
> +		.name		= "YUV 4:2:0 non-contiguous 3-planar,
Y/Cr/Cb",
>  		.pixelformat	= V4L2_PIX_FMT_YVU420M,
> -		.depth		= { 8, 2, 2 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CRCB,
>  		.num_planes	= 3,
>  		.num_comp	= 3,
> +		.depth		= { 8, 8, 8 },
> +		.sampling	= { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>  	}, {
> -		.name		= "YUV 4:2:0 n.c. 2p, Y/CbCr tiled",
> +		.name		=
> +			"YUV 4:2:0 non-contiguous 2-planar, Y/CbCr, tiled",
>  		.pixelformat	= V4L2_PIX_FMT_NV12MT_16X16,
> -		.depth		= { 8, 4 },
>  		.color		= GSC_YUV420,
>  		.yorder		= GSC_LSB_Y,
>  		.corder		= GSC_CBCR,
>  		.num_planes	= 2,
>  		.num_comp	= 2,
> -	}
> +		.depth		= { 8, 16 },
> +		.sampling	= { { 1, 1 }, { 2, 2 } },
> +	},
>  };
> 
>  const struct gsc_fmt *get_format(int index) @@ -384,6 +402,14 @@ void
> gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
>  			f_chk_addr, f_chk_len, s_chk_addr, s_chk_len);  }
> 
> +static void get_format_size(__u32 width, __u32 height,
> +			    const struct gsc_fmt *fmt, int plane,
> +			    __u16 *bytesperline, __u32 *sizeimage) {
> +	__u16 bpl = ((width * fmt->depth[plane]) / fmt-
> >sampling[plane][0]) / 8;
> +	*bytesperline = bpl;
> +	*sizeimage = (height * bpl) / fmt->sampling[plane][0]; }
> +
>  int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)  {
>  	struct gsc_dev *gsc = ctx->gsc_dev;
> @@ -448,14 +474,16 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx,
> struct v4l2_format *f)
>  	else /* SD */
>  		pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
> 
> -
> +	/* V4L2 specifies for contiguous planar formats that bytesperline
> and
> +	   sizeimage are set to values appropriate for the first plane.
> */
>  	for (i = 0; i < pix_mp->num_planes; ++i) {
> -		int bpl = (pix_mp->width * fmt->depth[i]) >> 3;
> -		pix_mp->plane_fmt[i].bytesperline = bpl;
> -		pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height;
> +		get_format_size(pix_mp->width, pix_mp->height, fmt, i,
> +				&pix_mp->plane_fmt[i].bytesperline,
> +				&pix_mp->plane_fmt[i].sizeimage);
> 
>  		pr_debug("[%d]: bpl: %d, sizeimage: %d",
> -				i, bpl, pix_mp->plane_fmt[i].sizeimage);
> +				i, pix_mp->plane_fmt[i].bytesperline,
> +				pix_mp->plane_fmt[i].sizeimage);
>  	}
> 
>  	return 0;
> @@ -465,26 +493,27 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct
> v4l2_format *f)  {
>  	struct gsc_frame *frame;
>  	struct v4l2_pix_format_mplane *pix_mp;
> +	const struct gsc_fmt *fmt;
>  	int i;
> 
>  	frame = ctx_get_frame(ctx, f->type);
>  	if (IS_ERR(frame))
>  		return PTR_ERR(frame);
> +	fmt = frame->fmt;
> 
>  	pix_mp = &f->fmt.pix_mp;
> 
>  	pix_mp->width		= frame->f_width;
>  	pix_mp->height		= frame->f_height;
>  	pix_mp->field		= V4L2_FIELD_NONE;
> -	pix_mp->pixelformat	= frame->fmt->pixelformat;
> +	pix_mp->pixelformat	= fmt->pixelformat;
>  	pix_mp->colorspace	= V4L2_COLORSPACE_REC709;
> -	pix_mp->num_planes	= frame->fmt->num_planes;
> +	pix_mp->num_planes	= fmt->num_planes;
> 
>  	for (i = 0; i < pix_mp->num_planes; ++i) {
> -		pix_mp->plane_fmt[i].bytesperline = (frame->f_width *
> -			frame->fmt->depth[i]) / 8;
> -		pix_mp->plane_fmt[i].sizeimage =
> -			 pix_mp->plane_fmt[i].bytesperline * frame-
>f_height;
> +		get_format_size(pix_mp->width, pix_mp->height, fmt, i,
> +				&pix_mp->plane_fmt[i].bytesperline,
> +				&pix_mp->plane_fmt[i].sizeimage);
>  	}
> 
>  	return 0;
> @@ -794,11 +823,12 @@ void gsc_ctrls_delete(struct gsc_ctx *ctx)
>  	}
>  }
> 
> -/* The color format (num_comp, num_planes) must be already configured.
> */
> +/* The color format (num_planes, num_comp) must be already configured.
> +*/
>  int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
>  			struct gsc_frame *frame, struct gsc_addr *addr)  {
>  	int ret = 0;
> +	const struct gsc_fmt *fmt = frame->fmt;
>  	u32 pix_size;
> 
>  	if ((vb == NULL) || (frame == NULL))
> @@ -810,46 +840,30 @@ int gsc_prepare_addr(struct gsc_ctx *ctx, struct
> vb2_buffer *vb,
>  		frame->fmt->num_planes, frame->fmt->num_comp, pix_size);
> 
>  	addr->y = vb2_dma_contig_plane_dma_addr(vb, 0);
> -
> -	if (frame->fmt->num_planes == 1) {
> -		switch (frame->fmt->num_comp) {
> -		case 1:
> -			addr->cb = 0;
> -			addr->cr = 0;
> -			break;
> -		case 2:
> -			/* decompose Y into Y/Cb */
> -			addr->cb = (dma_addr_t)(addr->y + pix_size);
> -			addr->cr = 0;
> -			break;
> -		case 3:
> -			/* decompose Y into Y/Cb/Cr */
> -			addr->cb = (dma_addr_t)(addr->y + pix_size);
> -			if (GSC_YUV420 == frame->fmt->color)
> -				addr->cr = (dma_addr_t)(addr->cb
> -						+ (pix_size >> 2));
> -			else /* 422 */
> -				addr->cr = (dma_addr_t)(addr->cb
> -						+ (pix_size >> 1));
> -			break;
> -		default:
> -			pr_err("Invalid the number of color planes");
> -			return -EINVAL;
> +	addr->cb = 0;
> +	addr->cr = 0;
> +
> +	if (fmt->num_planes == 1) {
> +		if (fmt->num_comp >= 2) {
> +			addr->cb = (dma_addr_t)(addr->y +
> +				((pix_size * fmt->depth[0]) /
> +				(fmt->sampling[0][0] *
> +				 fmt->sampling[0][1]) / 8));
> +		}
> +		if (fmt->num_comp >= 3) {
> +			addr->cr = (dma_addr_t)(addr->cb +
> +				((pix_size * fmt->depth[1]) /
> +				(fmt->sampling[1][0] *
> +				 fmt->sampling[1][1]) / 8));
>  		}
>  	} else {
> -		if (frame->fmt->num_planes >= 2)
> +		if (fmt->num_comp >= 2)
>  			addr->cb = vb2_dma_contig_plane_dma_addr(vb, 1);
> -
> -		if (frame->fmt->num_planes == 3)
> +		if (fmt->num_comp == 3)
>  			addr->cr = vb2_dma_contig_plane_dma_addr(vb, 2);
>  	}
> 
> -	if ((frame->fmt->pixelformat == V4L2_PIX_FMT_VYUY) ||
> -		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVYU) ||
> -		(frame->fmt->pixelformat == V4L2_PIX_FMT_NV61) ||
> -		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420) ||
> -		(frame->fmt->pixelformat == V4L2_PIX_FMT_NV21) ||
> -		(frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420M))
> +	if (fmt->corder == GSC_CRCB)
>  		swap(addr->cb, addr->cr);
> 
>  	pr_debug("ADDR: y= 0x%X  cb= 0x%X cr= 0x%X ret= %d", diff --git
> a/drivers/media/platform/exynos-gsc/gsc-core.h
> b/drivers/media/platform/exynos-gsc/gsc-core.h
> index ef0a6564..8fb07e0d 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -106,27 +106,27 @@ enum gsc_yuv_fmt {
>  	container_of((__ctrl)->handler, struct gsc_ctx, ctrl_handler)
>  /**
>   * struct gsc_fmt - the driver's internal color format data
> - * @mbus_code: Media Bus pixel code, -1 if not applicable
>   * @name: format description
>   * @pixelformat: the fourcc code for this format, 0 if not applicable
>   * @yorder: Y/C order
>   * @corder: Chrominance order control
>   * @num_planes: number of physically non-contiguous data planes
> - * @nr_comp: number of physically contiguous data planes
> - * @depth: per plane driver's private 'number of bits per pixel'
> - * @flags: flags indicating which operation mode format applies to
> + * @num_comp: number of physically contiguous data planes
> + * @depth: bit depth of each component
> + * @sampling: sampling frequency of each components, X and Y
> + * @mbus_code: Media Bus pixel code, -1 if not applicable
>   */
>  struct gsc_fmt {
> -	enum v4l2_mbus_pixelcode mbus_code;
>  	char	*name;
>  	u32	pixelformat;
>  	u32	color;
>  	u32	yorder;
>  	u32	corder;
> -	u16	num_planes;
> -	u16	num_comp;
> +	u8	num_planes;
> +	u8	num_comp;
>  	u8	depth[VIDEO_MAX_PLANES];
> -	u32	flags;
> +	u8	sampling[VIDEO_MAX_PLANES][2];
> +	enum v4l2_mbus_pixelcode mbus_code;
>  };
> 
>  /**
> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.c
> b/drivers/media/platform/exynos-gsc/gsc-regs.c
> index e22d147a..a8d6c90b 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-regs.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.c
> @@ -167,6 +167,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx
> *ctx)  {
>  	struct gsc_dev *dev = ctx->gsc_dev;
>  	struct gsc_frame *frame = &ctx->s_frame;
> +	const struct gsc_fmt *fmt = frame->fmt;
>  	u32 i, depth = 0;
>  	u32 cfg;
> 
> @@ -176,21 +177,22 @@ void gsc_hw_set_in_image_format(struct gsc_ctx
> *ctx)
>  		 GSC_IN_TILE_TYPE_MASK | GSC_IN_TILE_MODE);
>  	writel(cfg, dev->regs + GSC_IN_CON);
> 
> -	if (is_rgb(frame->fmt->color)) {
> +	if (is_rgb(fmt->color)) {
>  		gsc_hw_set_in_image_rgb(ctx);
>  		return;
>  	}
> -	for (i = 0; i < frame->fmt->num_planes; i++)
> -		depth += frame->fmt->depth[i];
> +	for (i = 0; i < fmt->num_comp; i++)
> +		depth += fmt->depth[i] /
> +			(fmt->sampling[i][0] * fmt->sampling[i][1]);
> 
> -	switch (frame->fmt->num_comp) {
> +	switch (fmt->num_comp) {
>  	case 1:
>  		cfg |= GSC_IN_YUV422_1P;
> -		if (frame->fmt->yorder == GSC_LSB_Y)
> +		if (fmt->yorder == GSC_LSB_Y)
>  			cfg |= GSC_IN_YUV422_1P_ORDER_LSB_Y;
>  		else
> -			cfg |= GSC_IN_YUV422_1P_OEDER_LSB_C;
> -		if (frame->fmt->corder == GSC_CBCR)
> +			cfg |= GSC_IN_YUV422_1P_ORDER_LSB_C;
> +		if (fmt->corder == GSC_CBCR)
>  			cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>  		else
>  			cfg |= GSC_IN_CHROMA_ORDER_CRCB;
> @@ -200,7 +202,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx
> *ctx)
>  			cfg |= GSC_IN_YUV420_2P;
>  		else
>  			cfg |= GSC_IN_YUV422_2P;
> -		if (frame->fmt->corder == GSC_CBCR)
> +		if (fmt->corder == GSC_CBCR)
>  			cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>  		else
>  			cfg |= GSC_IN_CHROMA_ORDER_CRCB;
> @@ -213,7 +215,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx
> *ctx)
>  		break;
>  	}
> 
> -	if (is_tiled(frame->fmt))
> +	if (is_tiled(fmt))
>  		cfg |= GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE;
> 
>  	writel(cfg, dev->regs + GSC_IN_CON);
> @@ -287,6 +289,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx
> *ctx)  {
>  	struct gsc_dev *dev = ctx->gsc_dev;
>  	struct gsc_frame *frame = &ctx->d_frame;
> +	const struct gsc_fmt *fmt = frame->fmt;
>  	u32 i, depth = 0;
>  	u32 cfg;
> 
> @@ -296,7 +299,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx
> *ctx)
>  		 GSC_OUT_TILE_TYPE_MASK | GSC_OUT_TILE_MODE);
>  	writel(cfg, dev->regs + GSC_OUT_CON);
> 
> -	if (is_rgb(frame->fmt->color)) {
> +	if (is_rgb(fmt->color)) {
>  		gsc_hw_set_out_image_rgb(ctx);
>  		return;
>  	}
> @@ -306,17 +309,18 @@ void gsc_hw_set_out_image_format(struct gsc_ctx
> *ctx)
>  		goto end_set;
>  	}
> 
> -	for (i = 0; i < frame->fmt->num_planes; i++)
> -		depth += frame->fmt->depth[i];
> +	for (i = 0; i < fmt->num_comp; i++)
> +		depth += fmt->depth[i] /
> +			(fmt->sampling[i][0] * fmt->sampling[i][1]);
> 
> -	switch (frame->fmt->num_comp) {
> +	switch (fmt->num_comp) {
>  	case 1:
>  		cfg |= GSC_OUT_YUV422_1P;
> -		if (frame->fmt->yorder == GSC_LSB_Y)
> +		if (fmt->yorder == GSC_LSB_Y)
>  			cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_Y;
>  		else
> -			cfg |= GSC_OUT_YUV422_1P_OEDER_LSB_C;
> -		if (frame->fmt->corder == GSC_CBCR)
> +			cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_C;
> +		if (fmt->corder == GSC_CBCR)
>  			cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>  		else
>  			cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
> @@ -326,7 +330,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx
> *ctx)
>  			cfg |= GSC_OUT_YUV420_2P;
>  		else
>  			cfg |= GSC_OUT_YUV422_2P;
> -		if (frame->fmt->corder == GSC_CBCR)
> +		if (fmt->corder == GSC_CBCR)
>  			cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>  		else
>  			cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
> @@ -336,7 +340,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx
> *ctx)
>  		break;
>  	}
> 
> -	if (is_tiled(frame->fmt))
> +	if (is_tiled(fmt))
>  		cfg |= GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE;
> 
>  end_set:
> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.h
> b/drivers/media/platform/exynos-gsc/gsc-regs.h
> index 4678f9a6..b03401dc 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-regs.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.h
> @@ -46,7 +46,7 @@
>  #define GSC_IN_RGB_SD_WIDE		(0 << 14)
>  #define GSC_IN_YUV422_1P_ORDER_MASK	(1 << 13)
>  #define GSC_IN_YUV422_1P_ORDER_LSB_Y	(0 << 13)
> -#define GSC_IN_YUV422_1P_OEDER_LSB_C	(1 << 13)
> +#define GSC_IN_YUV422_1P_ORDER_LSB_C	(1 << 13)
>  #define GSC_IN_CHROMA_ORDER_MASK	(1 << 12)
>  #define GSC_IN_CHROMA_ORDER_CBCR	(0 << 12)
>  #define GSC_IN_CHROMA_ORDER_CRCB	(1 << 12)
> @@ -91,7 +91,7 @@
>  #define GSC_OUT_RGB_SD_NARROW		(0 << 10)
>  #define GSC_OUT_YUV422_1P_ORDER_MASK	(1 << 9)
>  #define GSC_OUT_YUV422_1P_ORDER_LSB_Y	(0 << 9)
> -#define GSC_OUT_YUV422_1P_OEDER_LSB_C	(1 << 9)
> +#define GSC_OUT_YUV422_1P_ORDER_LSB_C	(1 << 9)
>  #define GSC_OUT_CHROMA_ORDER_MASK	(1 << 8)
>  #define GSC_OUT_CHROMA_ORDER_CBCR	(0 << 8)
>  #define GSC_OUT_CHROMA_ORDER_CRCB	(1 << 8)
> --
> 1.9.0.279.gdc9e3eb

Re: [PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage

[Shaik Ameer Basha]

Hi John Sheu,

Thanks for the patch.
Please find the review comments inline.

On Wed, Mar 12, 2014 at 4:22 AM, John Sheu <sheu@google.com> wrote:
> Explicitly specify sampling period for subsampled chroma formats, so
> stride and image size are properly reported through VIDIOC_{S,G}_FMT.
>
> Signed-off-by: John Sheu <sheu@google.com>
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.c | 154 +++++++++++++++------------
>  drivers/media/platform/exynos-gsc/gsc-core.h |  16 +--
>  drivers/media/platform/exynos-gsc/gsc-regs.c |  40 +++----
>  drivers/media/platform/exynos-gsc/gsc-regs.h |   4 +-
>  4 files changed, 116 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
> index 9d0cc04d..c02addef 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
> @@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
>         {
>                 .name           = "RGB565",
>                 .pixelformat    = V4L2_PIX_FMT_RGB565X,
> -               .depth          = { 16 },
>                 .color          = GSC_RGB,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 16 },
> +               .sampling       = { { 1, 1 } },
>         }, {
>                 .name           = "XRGB-8-8-8-8, 32 bpp",
>                 .pixelformat    = V4L2_PIX_FMT_RGB32,
> -               .depth          = { 32 },
>                 .color          = GSC_RGB,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 32 },
> +               .sampling       = { { 1, 1 } },
>         }, {
>                 .name           = "YUV 4:2:2 packed, YCbYCr",
>                 .pixelformat    = V4L2_PIX_FMT_YUYV,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 16 },
> +               .sampling       = { { 1, 1 } },
>                 .mbus_code      = V4L2_MBUS_FMT_YUYV8_2X8,
>         }, {
>                 .name           = "YUV 4:2:2 packed, CbYCrY",
>                 .pixelformat    = V4L2_PIX_FMT_UYVY,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_C,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 16 },
> +               .sampling       = { { 1, 1 } },
>                 .mbus_code      = V4L2_MBUS_FMT_UYVY8_2X8,
>         }, {
>                 .name           = "YUV 4:2:2 packed, CrYCbY",
>                 .pixelformat    = V4L2_PIX_FMT_VYUY,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_C,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 16 },
> +               .sampling       = { { 1, 1 } },
>                 .mbus_code      = V4L2_MBUS_FMT_VYUY8_2X8,
>         }, {
>                 .name           = "YUV 4:2:2 packed, YCrYCb",
>                 .pixelformat    = V4L2_PIX_FMT_YVYU,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 16 },
> +               .sampling       = { { 1, 1 } },
>                 .mbus_code      = V4L2_MBUS_FMT_YVYU8_2X8,
>         }, {
>                 .name           = "YUV 4:4:4 planar, YCbYCr",
>                 .pixelformat    = V4L2_PIX_FMT_YUV32,
> -               .depth          = { 32 },
>                 .color          = GSC_YUV444,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 1,
> +               .depth          = { 32 },
> +               .sampling       = { { 1, 1 } },
>         }, {
>                 .name           = "YUV 4:2:2 planar, Y/Cb/Cr",
>                 .pixelformat    = V4L2_PIX_FMT_YUV422P,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 3,
> +               .depth          = { 8, 8, 8 },
> +               .sampling       = { { 1, 1 }, { 2, 1 }, { 2, 1 } },
>         }, {
>                 .name           = "YUV 4:2:2 planar, Y/CbCr",
>                 .pixelformat    = V4L2_PIX_FMT_NV16,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 2,
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 1 } },
>         }, {
>                 .name           = "YUV 4:2:2 planar, Y/CrCb",
>                 .pixelformat    = V4L2_PIX_FMT_NV61,
> -               .depth          = { 16 },
>                 .color          = GSC_YUV422,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 1,
>                 .num_comp       = 2,
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 1 } },
>         }, {
>                 .name           = "YUV 4:2:0 planar, YCbCr",
>                 .pixelformat    = V4L2_PIX_FMT_YUV420,
> -               .depth          = { 12 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 3,
> +               .depth          = { 8, 8, 8 },
> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>         }, {
>                 .name           = "YUV 4:2:0 planar, YCrCb",
>                 .pixelformat    = V4L2_PIX_FMT_YVU420,
> -               .depth          = { 12 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 1,
>                 .num_comp       = 3,
> -
> +               .depth          = { 8, 8, 8 },
> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>         }, {
>                 .name           = "YUV 4:2:0 planar, Y/CbCr",
>                 .pixelformat    = V4L2_PIX_FMT_NV12,
> -               .depth          = { 12 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 1,
>                 .num_comp       = 2,
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>         }, {
>                 .name           = "YUV 4:2:0 planar, Y/CrCb",
>                 .pixelformat    = V4L2_PIX_FMT_NV21,
> -               .depth          = { 12 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 1,
>                 .num_comp       = 2,
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>         }, {
> -               .name           = "YUV 4:2:0 non-contig. 2p, Y/CbCr",
> +               .name           = "YUV 4:2:0 non-contiguous 2-planar, Y/CbCr",
>                 .pixelformat    = V4L2_PIX_FMT_NV12M,
> -               .depth          = { 8, 4 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 2,
>                 .num_comp       = 2,
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>         }, {
> -               .name           = "YUV 4:2:0 non-contig. 3p, Y/Cb/Cr",
> +               .name           = "YUV 4:2:0 non-contiguous 3-planar, Y/Cb/Cr",
>                 .pixelformat    = V4L2_PIX_FMT_YUV420M,
> -               .depth          = { 8, 2, 2 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 3,
>                 .num_comp       = 3,
> +               .depth          = { 8, 8, 8 },
> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>         }, {
> -               .name           = "YUV 4:2:0 non-contig. 3p, Y/Cr/Cb",
> +               .name           = "YUV 4:2:0 non-contiguous 3-planar, Y/Cr/Cb",
>                 .pixelformat    = V4L2_PIX_FMT_YVU420M,
> -               .depth          = { 8, 2, 2 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CRCB,
>                 .num_planes     = 3,
>                 .num_comp       = 3,
> +               .depth          = { 8, 8, 8 },
> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>         }, {
> -               .name           = "YUV 4:2:0 n.c. 2p, Y/CbCr tiled",
> +               .name           =
> +                       "YUV 4:2:0 non-contiguous 2-planar, Y/CbCr, tiled",
>                 .pixelformat    = V4L2_PIX_FMT_NV12MT_16X16,
> -               .depth          = { 8, 4 },
>                 .color          = GSC_YUV420,
>                 .yorder         = GSC_LSB_Y,
>                 .corder         = GSC_CBCR,
>                 .num_planes     = 2,
>                 .num_comp       = 2,
> -       }
> +               .depth          = { 8, 16 },
> +               .sampling       = { { 1, 1 }, { 2, 2 } },
> +       },
>  };
>
>  const struct gsc_fmt *get_format(int index)
> @@ -384,6 +402,14 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
>                         f_chk_addr, f_chk_len, s_chk_addr, s_chk_len);
>  }
>
> +static void get_format_size(__u32 width, __u32 height,
> +                           const struct gsc_fmt *fmt, int plane,
> +                           __u16 *bytesperline, __u32 *sizeimage) {
> +       __u16 bpl = ((width * fmt->depth[plane]) / fmt->sampling[plane][0]) / 8;
> +       *bytesperline = bpl;
> +       *sizeimage = (height * bpl) / fmt->sampling[plane][0];

should use the height sampling instead ??
*sizeimage = (height * bpl) / fmt->sampling[plane][1];

> +}
> +
>  int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>  {
>         struct gsc_dev *gsc = ctx->gsc_dev;
> @@ -448,14 +474,16 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>         else /* SD */
>                 pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
>
> -
> +       /* V4L2 specifies for contiguous planar formats that bytesperline and
> +          sizeimage are set to values appropriate for the first plane. */
>         for (i = 0; i < pix_mp->num_planes; ++i) {
> -               int bpl = (pix_mp->width * fmt->depth[i]) >> 3;
> -               pix_mp->plane_fmt[i].bytesperline = bpl;
> -               pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height;
> +               get_format_size(pix_mp->width, pix_mp->height, fmt, i,
> +                               &pix_mp->plane_fmt[i].bytesperline,
> +                               &pix_mp->plane_fmt[i].sizeimage);
>
>                 pr_debug("[%d]: bpl: %d, sizeimage: %d",
> -                               i, bpl, pix_mp->plane_fmt[i].sizeimage);
> +                               i, pix_mp->plane_fmt[i].bytesperline,
> +                               pix_mp->plane_fmt[i].sizeimage);
>         }
>
>         return 0;
> @@ -465,26 +493,27 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>  {
>         struct gsc_frame *frame;
>         struct v4l2_pix_format_mplane *pix_mp;
> +       const struct gsc_fmt *fmt;
>         int i;
>
>         frame = ctx_get_frame(ctx, f->type);
>         if (IS_ERR(frame))
>                 return PTR_ERR(frame);
> +       fmt = frame->fmt;
>
>         pix_mp = &f->fmt.pix_mp;
>
>         pix_mp->width           = frame->f_width;
>         pix_mp->height          = frame->f_height;
>         pix_mp->field           = V4L2_FIELD_NONE;
> -       pix_mp->pixelformat     = frame->fmt->pixelformat;
> +       pix_mp->pixelformat     = fmt->pixelformat;
>         pix_mp->colorspace      = V4L2_COLORSPACE_REC709;
> -       pix_mp->num_planes      = frame->fmt->num_planes;
> +       pix_mp->num_planes      = fmt->num_planes;
>
>         for (i = 0; i < pix_mp->num_planes; ++i) {
> -               pix_mp->plane_fmt[i].bytesperline = (frame->f_width *
> -                       frame->fmt->depth[i]) / 8;
> -               pix_mp->plane_fmt[i].sizeimage =
> -                        pix_mp->plane_fmt[i].bytesperline * frame->f_height;
> +               get_format_size(pix_mp->width, pix_mp->height, fmt, i,
> +                               &pix_mp->plane_fmt[i].bytesperline,
> +                               &pix_mp->plane_fmt[i].sizeimage);
>         }
>
>         return 0;
> @@ -794,11 +823,12 @@ void gsc_ctrls_delete(struct gsc_ctx *ctx)
>         }
>  }
>
> -/* The color format (num_comp, num_planes) must be already configured. */
> +/* The color format (num_planes, num_comp) must be already configured. */
>  int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
>                         struct gsc_frame *frame, struct gsc_addr *addr)
>  {
>         int ret = 0;
> +       const struct gsc_fmt *fmt = frame->fmt;
>         u32 pix_size;
>
>         if ((vb == NULL) || (frame == NULL))
> @@ -810,46 +840,30 @@ int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
>                 frame->fmt->num_planes, frame->fmt->num_comp, pix_size);
>
>         addr->y = vb2_dma_contig_plane_dma_addr(vb, 0);
> -
> -       if (frame->fmt->num_planes == 1) {
> -               switch (frame->fmt->num_comp) {
> -               case 1:
> -                       addr->cb = 0;
> -                       addr->cr = 0;
> -                       break;
> -               case 2:
> -                       /* decompose Y into Y/Cb */
> -                       addr->cb = (dma_addr_t)(addr->y + pix_size);
> -                       addr->cr = 0;
> -                       break;
> -               case 3:
> -                       /* decompose Y into Y/Cb/Cr */
> -                       addr->cb = (dma_addr_t)(addr->y + pix_size);
> -                       if (GSC_YUV420 == frame->fmt->color)
> -                               addr->cr = (dma_addr_t)(addr->cb
> -                                               + (pix_size >> 2));
> -                       else /* 422 */
> -                               addr->cr = (dma_addr_t)(addr->cb
> -                                               + (pix_size >> 1));
> -                       break;
> -               default:
> -                       pr_err("Invalid the number of color planes");
> -                       return -EINVAL;
> +       addr->cb = 0;
> +       addr->cr = 0;
> +
> +       if (fmt->num_planes == 1) {
> +               if (fmt->num_comp >= 2) {
> +                       addr->cb = (dma_addr_t)(addr->y +
> +                               ((pix_size * fmt->depth[0]) /
> +                               (fmt->sampling[0][0] *
> +                                fmt->sampling[0][1]) / 8));
> +               }
> +               if (fmt->num_comp >= 3) {
> +                       addr->cr = (dma_addr_t)(addr->cb +
> +                               ((pix_size * fmt->depth[1]) /
> +                               (fmt->sampling[1][0] *
> +                                fmt->sampling[1][1]) / 8));
>                 }
>         } else {
> -               if (frame->fmt->num_planes >= 2)
> +               if (fmt->num_comp >= 2)
>                         addr->cb = vb2_dma_contig_plane_dma_addr(vb, 1);
> -
> -               if (frame->fmt->num_planes == 3)
> +               if (fmt->num_comp == 3)
>                         addr->cr = vb2_dma_contig_plane_dma_addr(vb, 2);
>         }
>
> -       if ((frame->fmt->pixelformat == V4L2_PIX_FMT_VYUY) ||
> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVYU) ||
> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_NV61) ||
> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420) ||
> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_NV21) ||
> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420M))
> +       if (fmt->corder == GSC_CRCB)

            if ((fmt->corder == GSC_CRCB) && (fmt->num_planes == 3))

I think it shoud only applicable to 3 component formats.
for example, Incase of two component formats like V4L2_PIX_FMT_NV61,
chroma base address becomes zero.


Regards,
Shaik Ameer Basha

>                 swap(addr->cb, addr->cr);
>
>         pr_debug("ADDR: y= 0x%X  cb= 0x%X cr= 0x%X ret= %d",
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
> index ef0a6564..8fb07e0d 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -106,27 +106,27 @@ enum gsc_yuv_fmt {
>         container_of((__ctrl)->handler, struct gsc_ctx, ctrl_handler)
>  /**
>   * struct gsc_fmt - the driver's internal color format data
> - * @mbus_code: Media Bus pixel code, -1 if not applicable
>   * @name: format description
>   * @pixelformat: the fourcc code for this format, 0 if not applicable
>   * @yorder: Y/C order
>   * @corder: Chrominance order control
>   * @num_planes: number of physically non-contiguous data planes
> - * @nr_comp: number of physically contiguous data planes
> - * @depth: per plane driver's private 'number of bits per pixel'
> - * @flags: flags indicating which operation mode format applies to
> + * @num_comp: number of physically contiguous data planes
> + * @depth: bit depth of each component
> + * @sampling: sampling frequency of each components, X and Y
> + * @mbus_code: Media Bus pixel code, -1 if not applicable
>   */
>  struct gsc_fmt {
> -       enum v4l2_mbus_pixelcode mbus_code;
>         char    *name;
>         u32     pixelformat;
>         u32     color;
>         u32     yorder;
>         u32     corder;
> -       u16     num_planes;
> -       u16     num_comp;
> +       u8      num_planes;
> +       u8      num_comp;
>         u8      depth[VIDEO_MAX_PLANES];
> -       u32     flags;
> +       u8      sampling[VIDEO_MAX_PLANES][2];
> +       enum v4l2_mbus_pixelcode mbus_code;
>  };
>
>  /**
> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.c b/drivers/media/platform/exynos-gsc/gsc-regs.c
> index e22d147a..a8d6c90b 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-regs.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.c
> @@ -167,6 +167,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>  {
>         struct gsc_dev *dev = ctx->gsc_dev;
>         struct gsc_frame *frame = &ctx->s_frame;
> +       const struct gsc_fmt *fmt = frame->fmt;
>         u32 i, depth = 0;
>         u32 cfg;
>
> @@ -176,21 +177,22 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>                  GSC_IN_TILE_TYPE_MASK | GSC_IN_TILE_MODE);
>         writel(cfg, dev->regs + GSC_IN_CON);
>
> -       if (is_rgb(frame->fmt->color)) {
> +       if (is_rgb(fmt->color)) {
>                 gsc_hw_set_in_image_rgb(ctx);
>                 return;
>         }
> -       for (i = 0; i < frame->fmt->num_planes; i++)
> -               depth += frame->fmt->depth[i];
> +       for (i = 0; i < fmt->num_comp; i++)
> +               depth += fmt->depth[i] /
> +                       (fmt->sampling[i][0] * fmt->sampling[i][1]);
>
> -       switch (frame->fmt->num_comp) {
> +       switch (fmt->num_comp) {
>         case 1:
>                 cfg |= GSC_IN_YUV422_1P;
> -               if (frame->fmt->yorder == GSC_LSB_Y)
> +               if (fmt->yorder == GSC_LSB_Y)
>                         cfg |= GSC_IN_YUV422_1P_ORDER_LSB_Y;
>                 else
> -                       cfg |= GSC_IN_YUV422_1P_OEDER_LSB_C;
> -               if (frame->fmt->corder == GSC_CBCR)
> +                       cfg |= GSC_IN_YUV422_1P_ORDER_LSB_C;
> +               if (fmt->corder == GSC_CBCR)
>                         cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>                 else
>                         cfg |= GSC_IN_CHROMA_ORDER_CRCB;
> @@ -200,7 +202,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>                         cfg |= GSC_IN_YUV420_2P;
>                 else
>                         cfg |= GSC_IN_YUV422_2P;
> -               if (frame->fmt->corder == GSC_CBCR)
> +               if (fmt->corder == GSC_CBCR)
>                         cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>                 else
>                         cfg |= GSC_IN_CHROMA_ORDER_CRCB;
> @@ -213,7 +215,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>                 break;
>         }
>
> -       if (is_tiled(frame->fmt))
> +       if (is_tiled(fmt))
>                 cfg |= GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE;
>
>         writel(cfg, dev->regs + GSC_IN_CON);
> @@ -287,6 +289,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>  {
>         struct gsc_dev *dev = ctx->gsc_dev;
>         struct gsc_frame *frame = &ctx->d_frame;
> +       const struct gsc_fmt *fmt = frame->fmt;
>         u32 i, depth = 0;
>         u32 cfg;
>
> @@ -296,7 +299,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>                  GSC_OUT_TILE_TYPE_MASK | GSC_OUT_TILE_MODE);
>         writel(cfg, dev->regs + GSC_OUT_CON);
>
> -       if (is_rgb(frame->fmt->color)) {
> +       if (is_rgb(fmt->color)) {
>                 gsc_hw_set_out_image_rgb(ctx);
>                 return;
>         }
> @@ -306,17 +309,18 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>                 goto end_set;
>         }
>
> -       for (i = 0; i < frame->fmt->num_planes; i++)
> -               depth += frame->fmt->depth[i];
> +       for (i = 0; i < fmt->num_comp; i++)
> +               depth += fmt->depth[i] /
> +                       (fmt->sampling[i][0] * fmt->sampling[i][1]);
>
> -       switch (frame->fmt->num_comp) {
> +       switch (fmt->num_comp) {
>         case 1:
>                 cfg |= GSC_OUT_YUV422_1P;
> -               if (frame->fmt->yorder == GSC_LSB_Y)
> +               if (fmt->yorder == GSC_LSB_Y)
>                         cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_Y;
>                 else
> -                       cfg |= GSC_OUT_YUV422_1P_OEDER_LSB_C;
> -               if (frame->fmt->corder == GSC_CBCR)
> +                       cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_C;
> +               if (fmt->corder == GSC_CBCR)
>                         cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>                 else
>                         cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
> @@ -326,7 +330,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>                         cfg |= GSC_OUT_YUV420_2P;
>                 else
>                         cfg |= GSC_OUT_YUV422_2P;
> -               if (frame->fmt->corder == GSC_CBCR)
> +               if (fmt->corder == GSC_CBCR)
>                         cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>                 else
>                         cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
> @@ -336,7 +340,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>                 break;
>         }
>
> -       if (is_tiled(frame->fmt))
> +       if (is_tiled(fmt))
>                 cfg |= GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE;
>
>  end_set:
> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.h b/drivers/media/platform/exynos-gsc/gsc-regs.h
> index 4678f9a6..b03401dc 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-regs.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.h
> @@ -46,7 +46,7 @@
>  #define GSC_IN_RGB_SD_WIDE             (0 << 14)
>  #define GSC_IN_YUV422_1P_ORDER_MASK    (1 << 13)
>  #define GSC_IN_YUV422_1P_ORDER_LSB_Y   (0 << 13)
> -#define GSC_IN_YUV422_1P_OEDER_LSB_C   (1 << 13)
> +#define GSC_IN_YUV422_1P_ORDER_LSB_C   (1 << 13)
>  #define GSC_IN_CHROMA_ORDER_MASK       (1 << 12)
>  #define GSC_IN_CHROMA_ORDER_CBCR       (0 << 12)
>  #define GSC_IN_CHROMA_ORDER_CRCB       (1 << 12)
> @@ -91,7 +91,7 @@
>  #define GSC_OUT_RGB_SD_NARROW          (0 << 10)
>  #define GSC_OUT_YUV422_1P_ORDER_MASK   (1 << 9)
>  #define GSC_OUT_YUV422_1P_ORDER_LSB_Y  (0 << 9)
> -#define GSC_OUT_YUV422_1P_OEDER_LSB_C  (1 << 9)
> +#define GSC_OUT_YUV422_1P_ORDER_LSB_C  (1 << 9)
>  #define GSC_OUT_CHROMA_ORDER_MASK      (1 << 8)
>  #define GSC_OUT_CHROMA_ORDER_CBCR      (0 << 8)
>  #define GSC_OUT_CHROMA_ORDER_CRCB      (1 << 8)
> --
> 1.9.0.279.gdc9e3eb
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] gsc-m2m: report correct format bytesperline and sizeimage

[Shaik Ameer Basha]

On Tue, Apr 8, 2014 at 12:26 PM, Shaik Ameer Basha
<shaik.samsung@gmail.com> wrote:
> Hi John Sheu,
>
> Thanks for the patch.
> Please find the review comments inline.
>
> On Wed, Mar 12, 2014 at 4:22 AM, John Sheu <sheu@google.com> wrote:
>> Explicitly specify sampling period for subsampled chroma formats, so
>> stride and image size are properly reported through VIDIOC_{S,G}_FMT.
>>
>> Signed-off-by: John Sheu <sheu@google.com>
>> ---
>>  drivers/media/platform/exynos-gsc/gsc-core.c | 154 +++++++++++++++------------
>>  drivers/media/platform/exynos-gsc/gsc-core.h |  16 +--
>>  drivers/media/platform/exynos-gsc/gsc-regs.c |  40 +++----
>>  drivers/media/platform/exynos-gsc/gsc-regs.h |   4 +-
>>  4 files changed, 116 insertions(+), 98 deletions(-)
>>
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c
>> index 9d0cc04d..c02addef 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-core.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-core.c
>> @@ -34,167 +34,185 @@ static const struct gsc_fmt gsc_formats[] = {
>>         {
>>                 .name           = "RGB565",
>>                 .pixelformat    = V4L2_PIX_FMT_RGB565X,
>> -               .depth          = { 16 },
>>                 .color          = GSC_RGB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 16 },
>> +               .sampling       = { { 1, 1 } },
>>         }, {
>>                 .name           = "XRGB-8-8-8-8, 32 bpp",
>>                 .pixelformat    = V4L2_PIX_FMT_RGB32,
>> -               .depth          = { 32 },
>>                 .color          = GSC_RGB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 32 },
>> +               .sampling       = { { 1, 1 } },
>>         }, {
>>                 .name           = "YUV 4:2:2 packed, YCbYCr",
>>                 .pixelformat    = V4L2_PIX_FMT_YUYV,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 16 },
>> +               .sampling       = { { 1, 1 } },
>>                 .mbus_code      = V4L2_MBUS_FMT_YUYV8_2X8,
>>         }, {
>>                 .name           = "YUV 4:2:2 packed, CbYCrY",
>>                 .pixelformat    = V4L2_PIX_FMT_UYVY,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_C,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 16 },
>> +               .sampling       = { { 1, 1 } },
>>                 .mbus_code      = V4L2_MBUS_FMT_UYVY8_2X8,
>>         }, {
>>                 .name           = "YUV 4:2:2 packed, CrYCbY",
>>                 .pixelformat    = V4L2_PIX_FMT_VYUY,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_C,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 16 },
>> +               .sampling       = { { 1, 1 } },
>>                 .mbus_code      = V4L2_MBUS_FMT_VYUY8_2X8,
>>         }, {
>>                 .name           = "YUV 4:2:2 packed, YCrYCb",
>>                 .pixelformat    = V4L2_PIX_FMT_YVYU,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 16 },
>> +               .sampling       = { { 1, 1 } },
>>                 .mbus_code      = V4L2_MBUS_FMT_YVYU8_2X8,
>>         }, {
>>                 .name           = "YUV 4:4:4 planar, YCbYCr",
>>                 .pixelformat    = V4L2_PIX_FMT_YUV32,
>> -               .depth          = { 32 },
>>                 .color          = GSC_YUV444,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 1,
>> +               .depth          = { 32 },
>> +               .sampling       = { { 1, 1 } },
>>         }, {
>>                 .name           = "YUV 4:2:2 planar, Y/Cb/Cr",
>>                 .pixelformat    = V4L2_PIX_FMT_YUV422P,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 3,
>> +               .depth          = { 8, 8, 8 },
>> +               .sampling       = { { 1, 1 }, { 2, 1 }, { 2, 1 } },
>>         }, {
>>                 .name           = "YUV 4:2:2 planar, Y/CbCr",
>>                 .pixelformat    = V4L2_PIX_FMT_NV16,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 2,
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 1 } },
>>         }, {
>>                 .name           = "YUV 4:2:2 planar, Y/CrCb",
>>                 .pixelformat    = V4L2_PIX_FMT_NV61,
>> -               .depth          = { 16 },
>>                 .color          = GSC_YUV422,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 2,
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 1 } },
>>         }, {
>>                 .name           = "YUV 4:2:0 planar, YCbCr",
>>                 .pixelformat    = V4L2_PIX_FMT_YUV420,
>> -               .depth          = { 12 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 3,
>> +               .depth          = { 8, 8, 8 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>>         }, {
>>                 .name           = "YUV 4:2:0 planar, YCrCb",
>>                 .pixelformat    = V4L2_PIX_FMT_YVU420,
>> -               .depth          = { 12 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 3,
>> -
>> +               .depth          = { 8, 8, 8 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>>         }, {
>>                 .name           = "YUV 4:2:0 planar, Y/CbCr",
>>                 .pixelformat    = V4L2_PIX_FMT_NV12,
>> -               .depth          = { 12 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 1,
>>                 .num_comp       = 2,
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>>         }, {
>>                 .name           = "YUV 4:2:0 planar, Y/CrCb",
>>                 .pixelformat    = V4L2_PIX_FMT_NV21,
>> -               .depth          = { 12 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 1,
>>                 .num_comp       = 2,
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>>         }, {
>> -               .name           = "YUV 4:2:0 non-contig. 2p, Y/CbCr",
>> +               .name           = "YUV 4:2:0 non-contiguous 2-planar, Y/CbCr",
>>                 .pixelformat    = V4L2_PIX_FMT_NV12M,
>> -               .depth          = { 8, 4 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 2,
>>                 .num_comp       = 2,
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>>         }, {
>> -               .name           = "YUV 4:2:0 non-contig. 3p, Y/Cb/Cr",
>> +               .name           = "YUV 4:2:0 non-contiguous 3-planar, Y/Cb/Cr",
>>                 .pixelformat    = V4L2_PIX_FMT_YUV420M,
>> -               .depth          = { 8, 2, 2 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 3,
>>                 .num_comp       = 3,
>> +               .depth          = { 8, 8, 8 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>>         }, {
>> -               .name           = "YUV 4:2:0 non-contig. 3p, Y/Cr/Cb",
>> +               .name           = "YUV 4:2:0 non-contiguous 3-planar, Y/Cr/Cb",
>>                 .pixelformat    = V4L2_PIX_FMT_YVU420M,
>> -               .depth          = { 8, 2, 2 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CRCB,
>>                 .num_planes     = 3,
>>                 .num_comp       = 3,
>> +               .depth          = { 8, 8, 8 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 }, { 2, 2 } },
>>         }, {
>> -               .name           = "YUV 4:2:0 n.c. 2p, Y/CbCr tiled",
>> +               .name           =
>> +                       "YUV 4:2:0 non-contiguous 2-planar, Y/CbCr, tiled",
>>                 .pixelformat    = V4L2_PIX_FMT_NV12MT_16X16,
>> -               .depth          = { 8, 4 },
>>                 .color          = GSC_YUV420,
>>                 .yorder         = GSC_LSB_Y,
>>                 .corder         = GSC_CBCR,
>>                 .num_planes     = 2,
>>                 .num_comp       = 2,
>> -       }
>> +               .depth          = { 8, 16 },
>> +               .sampling       = { { 1, 1 }, { 2, 2 } },
>> +       },
>>  };
>>
>>  const struct gsc_fmt *get_format(int index)
>> @@ -384,6 +402,14 @@ void gsc_set_prefbuf(struct gsc_dev *gsc, struct gsc_frame *frm)
>>                         f_chk_addr, f_chk_len, s_chk_addr, s_chk_len);
>>  }
>>
>> +static void get_format_size(__u32 width, __u32 height,
>> +                           const struct gsc_fmt *fmt, int plane,
>> +                           __u16 *bytesperline, __u32 *sizeimage) {
>> +       __u16 bpl = ((width * fmt->depth[plane]) / fmt->sampling[plane][0]) / 8;
>> +       *bytesperline = bpl;
>> +       *sizeimage = (height * bpl) / fmt->sampling[plane][0];
>
> should use the height sampling instead ??
> *sizeimage = (height * bpl) / fmt->sampling[plane][1];
>
>> +}
>> +
>>  int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>>  {
>>         struct gsc_dev *gsc = ctx->gsc_dev;
>> @@ -448,14 +474,16 @@ int gsc_try_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>>         else /* SD */
>>                 pix_mp->colorspace = V4L2_COLORSPACE_SMPTE170M;
>>
>> -
>> +       /* V4L2 specifies for contiguous planar formats that bytesperline and
>> +          sizeimage are set to values appropriate for the first plane. */
>>         for (i = 0; i < pix_mp->num_planes; ++i) {
>> -               int bpl = (pix_mp->width * fmt->depth[i]) >> 3;
>> -               pix_mp->plane_fmt[i].bytesperline = bpl;
>> -               pix_mp->plane_fmt[i].sizeimage = bpl * pix_mp->height;
>> +               get_format_size(pix_mp->width, pix_mp->height, fmt, i,
>> +                               &pix_mp->plane_fmt[i].bytesperline,
>> +                               &pix_mp->plane_fmt[i].sizeimage);
>>
>>                 pr_debug("[%d]: bpl: %d, sizeimage: %d",
>> -                               i, bpl, pix_mp->plane_fmt[i].sizeimage);
>> +                               i, pix_mp->plane_fmt[i].bytesperline,
>> +                               pix_mp->plane_fmt[i].sizeimage);
>>         }
>>
>>         return 0;
>> @@ -465,26 +493,27 @@ int gsc_g_fmt_mplane(struct gsc_ctx *ctx, struct v4l2_format *f)
>>  {
>>         struct gsc_frame *frame;
>>         struct v4l2_pix_format_mplane *pix_mp;
>> +       const struct gsc_fmt *fmt;
>>         int i;
>>
>>         frame = ctx_get_frame(ctx, f->type);
>>         if (IS_ERR(frame))
>>                 return PTR_ERR(frame);
>> +       fmt = frame->fmt;
>>
>>         pix_mp = &f->fmt.pix_mp;
>>
>>         pix_mp->width           = frame->f_width;
>>         pix_mp->height          = frame->f_height;
>>         pix_mp->field           = V4L2_FIELD_NONE;
>> -       pix_mp->pixelformat     = frame->fmt->pixelformat;
>> +       pix_mp->pixelformat     = fmt->pixelformat;
>>         pix_mp->colorspace      = V4L2_COLORSPACE_REC709;
>> -       pix_mp->num_planes      = frame->fmt->num_planes;
>> +       pix_mp->num_planes      = fmt->num_planes;
>>
>>         for (i = 0; i < pix_mp->num_planes; ++i) {
>> -               pix_mp->plane_fmt[i].bytesperline = (frame->f_width *
>> -                       frame->fmt->depth[i]) / 8;
>> -               pix_mp->plane_fmt[i].sizeimage =
>> -                        pix_mp->plane_fmt[i].bytesperline * frame->f_height;
>> +               get_format_size(pix_mp->width, pix_mp->height, fmt, i,
>> +                               &pix_mp->plane_fmt[i].bytesperline,
>> +                               &pix_mp->plane_fmt[i].sizeimage);
>>         }
>>
>>         return 0;
>> @@ -794,11 +823,12 @@ void gsc_ctrls_delete(struct gsc_ctx *ctx)
>>         }
>>  }
>>
>> -/* The color format (num_comp, num_planes) must be already configured. */
>> +/* The color format (num_planes, num_comp) must be already configured. */
>>  int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
>>                         struct gsc_frame *frame, struct gsc_addr *addr)
>>  {
>>         int ret = 0;
>> +       const struct gsc_fmt *fmt = frame->fmt;
>>         u32 pix_size;
>>
>>         if ((vb == NULL) || (frame == NULL))
>> @@ -810,46 +840,30 @@ int gsc_prepare_addr(struct gsc_ctx *ctx, struct vb2_buffer *vb,
>>                 frame->fmt->num_planes, frame->fmt->num_comp, pix_size);
>>
>>         addr->y = vb2_dma_contig_plane_dma_addr(vb, 0);
>> -
>> -       if (frame->fmt->num_planes == 1) {
>> -               switch (frame->fmt->num_comp) {
>> -               case 1:
>> -                       addr->cb = 0;
>> -                       addr->cr = 0;
>> -                       break;
>> -               case 2:
>> -                       /* decompose Y into Y/Cb */
>> -                       addr->cb = (dma_addr_t)(addr->y + pix_size);
>> -                       addr->cr = 0;
>> -                       break;
>> -               case 3:
>> -                       /* decompose Y into Y/Cb/Cr */
>> -                       addr->cb = (dma_addr_t)(addr->y + pix_size);
>> -                       if (GSC_YUV420 == frame->fmt->color)
>> -                               addr->cr = (dma_addr_t)(addr->cb
>> -                                               + (pix_size >> 2));
>> -                       else /* 422 */
>> -                               addr->cr = (dma_addr_t)(addr->cb
>> -                                               + (pix_size >> 1));
>> -                       break;
>> -               default:
>> -                       pr_err("Invalid the number of color planes");
>> -                       return -EINVAL;
>> +       addr->cb = 0;
>> +       addr->cr = 0;
>> +
>> +       if (fmt->num_planes == 1) {
>> +               if (fmt->num_comp >= 2) {
>> +                       addr->cb = (dma_addr_t)(addr->y +
>> +                               ((pix_size * fmt->depth[0]) /
>> +                               (fmt->sampling[0][0] *
>> +                                fmt->sampling[0][1]) / 8));
>> +               }
>> +               if (fmt->num_comp >= 3) {
>> +                       addr->cr = (dma_addr_t)(addr->cb +
>> +                               ((pix_size * fmt->depth[1]) /
>> +                               (fmt->sampling[1][0] *
>> +                                fmt->sampling[1][1]) / 8));
>>                 }
>>         } else {
>> -               if (frame->fmt->num_planes >= 2)
>> +               if (fmt->num_comp >= 2)
>>                         addr->cb = vb2_dma_contig_plane_dma_addr(vb, 1);
>> -
>> -               if (frame->fmt->num_planes == 3)
>> +               if (fmt->num_comp == 3)
>>                         addr->cr = vb2_dma_contig_plane_dma_addr(vb, 2);
>>         }
>>
>> -       if ((frame->fmt->pixelformat == V4L2_PIX_FMT_VYUY) ||
>> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVYU) ||
>> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_NV61) ||
>> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420) ||
>> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_NV21) ||
>> -               (frame->fmt->pixelformat == V4L2_PIX_FMT_YVU420M))
>> +       if (fmt->corder == GSC_CRCB)
>
>             if ((fmt->corder == GSC_CRCB) && (fmt->num_planes == 3))

sorry, it is....
               if ((fmt->corder == GSC_CRCB) && (fmt->num_comp == 3))

>
> I think it shoud only applicable to 3 component formats.
> for example, Incase of two component formats like V4L2_PIX_FMT_NV61,
> chroma base address becomes zero.
>
>
> Regards,
> Shaik Ameer Basha
>
>>                 swap(addr->cb, addr->cr);
>>
>>         pr_debug("ADDR: y= 0x%X  cb= 0x%X cr= 0x%X ret= %d",
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
>> index ef0a6564..8fb07e0d 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
>> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
>> @@ -106,27 +106,27 @@ enum gsc_yuv_fmt {
>>         container_of((__ctrl)->handler, struct gsc_ctx, ctrl_handler)
>>  /**
>>   * struct gsc_fmt - the driver's internal color format data
>> - * @mbus_code: Media Bus pixel code, -1 if not applicable
>>   * @name: format description
>>   * @pixelformat: the fourcc code for this format, 0 if not applicable
>>   * @yorder: Y/C order
>>   * @corder: Chrominance order control
>>   * @num_planes: number of physically non-contiguous data planes
>> - * @nr_comp: number of physically contiguous data planes
>> - * @depth: per plane driver's private 'number of bits per pixel'
>> - * @flags: flags indicating which operation mode format applies to
>> + * @num_comp: number of physically contiguous data planes
>> + * @depth: bit depth of each component
>> + * @sampling: sampling frequency of each components, X and Y
>> + * @mbus_code: Media Bus pixel code, -1 if not applicable
>>   */
>>  struct gsc_fmt {
>> -       enum v4l2_mbus_pixelcode mbus_code;
>>         char    *name;
>>         u32     pixelformat;
>>         u32     color;
>>         u32     yorder;
>>         u32     corder;
>> -       u16     num_planes;
>> -       u16     num_comp;
>> +       u8      num_planes;
>> +       u8      num_comp;
>>         u8      depth[VIDEO_MAX_PLANES];
>> -       u32     flags;
>> +       u8      sampling[VIDEO_MAX_PLANES][2];
>> +       enum v4l2_mbus_pixelcode mbus_code;
>>  };
>>
>>  /**
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.c b/drivers/media/platform/exynos-gsc/gsc-regs.c
>> index e22d147a..a8d6c90b 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-regs.c
>> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.c
>> @@ -167,6 +167,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>>  {
>>         struct gsc_dev *dev = ctx->gsc_dev;
>>         struct gsc_frame *frame = &ctx->s_frame;
>> +       const struct gsc_fmt *fmt = frame->fmt;
>>         u32 i, depth = 0;
>>         u32 cfg;
>>
>> @@ -176,21 +177,22 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>>                  GSC_IN_TILE_TYPE_MASK | GSC_IN_TILE_MODE);
>>         writel(cfg, dev->regs + GSC_IN_CON);
>>
>> -       if (is_rgb(frame->fmt->color)) {
>> +       if (is_rgb(fmt->color)) {
>>                 gsc_hw_set_in_image_rgb(ctx);
>>                 return;
>>         }
>> -       for (i = 0; i < frame->fmt->num_planes; i++)
>> -               depth += frame->fmt->depth[i];
>> +       for (i = 0; i < fmt->num_comp; i++)
>> +               depth += fmt->depth[i] /
>> +                       (fmt->sampling[i][0] * fmt->sampling[i][1]);
>>
>> -       switch (frame->fmt->num_comp) {
>> +       switch (fmt->num_comp) {
>>         case 1:
>>                 cfg |= GSC_IN_YUV422_1P;
>> -               if (frame->fmt->yorder == GSC_LSB_Y)
>> +               if (fmt->yorder == GSC_LSB_Y)
>>                         cfg |= GSC_IN_YUV422_1P_ORDER_LSB_Y;
>>                 else
>> -                       cfg |= GSC_IN_YUV422_1P_OEDER_LSB_C;
>> -               if (frame->fmt->corder == GSC_CBCR)
>> +                       cfg |= GSC_IN_YUV422_1P_ORDER_LSB_C;
>> +               if (fmt->corder == GSC_CBCR)
>>                         cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>>                 else
>>                         cfg |= GSC_IN_CHROMA_ORDER_CRCB;
>> @@ -200,7 +202,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>>                         cfg |= GSC_IN_YUV420_2P;
>>                 else
>>                         cfg |= GSC_IN_YUV422_2P;
>> -               if (frame->fmt->corder == GSC_CBCR)
>> +               if (fmt->corder == GSC_CBCR)
>>                         cfg |= GSC_IN_CHROMA_ORDER_CBCR;
>>                 else
>>                         cfg |= GSC_IN_CHROMA_ORDER_CRCB;
>> @@ -213,7 +215,7 @@ void gsc_hw_set_in_image_format(struct gsc_ctx *ctx)
>>                 break;
>>         }
>>
>> -       if (is_tiled(frame->fmt))
>> +       if (is_tiled(fmt))
>>                 cfg |= GSC_IN_TILE_C_16x8 | GSC_IN_TILE_MODE;
>>
>>         writel(cfg, dev->regs + GSC_IN_CON);
>> @@ -287,6 +289,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>>  {
>>         struct gsc_dev *dev = ctx->gsc_dev;
>>         struct gsc_frame *frame = &ctx->d_frame;
>> +       const struct gsc_fmt *fmt = frame->fmt;
>>         u32 i, depth = 0;
>>         u32 cfg;
>>
>> @@ -296,7 +299,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>>                  GSC_OUT_TILE_TYPE_MASK | GSC_OUT_TILE_MODE);
>>         writel(cfg, dev->regs + GSC_OUT_CON);
>>
>> -       if (is_rgb(frame->fmt->color)) {
>> +       if (is_rgb(fmt->color)) {
>>                 gsc_hw_set_out_image_rgb(ctx);
>>                 return;
>>         }
>> @@ -306,17 +309,18 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>>                 goto end_set;
>>         }
>>
>> -       for (i = 0; i < frame->fmt->num_planes; i++)
>> -               depth += frame->fmt->depth[i];
>> +       for (i = 0; i < fmt->num_comp; i++)
>> +               depth += fmt->depth[i] /
>> +                       (fmt->sampling[i][0] * fmt->sampling[i][1]);
>>
>> -       switch (frame->fmt->num_comp) {
>> +       switch (fmt->num_comp) {
>>         case 1:
>>                 cfg |= GSC_OUT_YUV422_1P;
>> -               if (frame->fmt->yorder == GSC_LSB_Y)
>> +               if (fmt->yorder == GSC_LSB_Y)
>>                         cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_Y;
>>                 else
>> -                       cfg |= GSC_OUT_YUV422_1P_OEDER_LSB_C;
>> -               if (frame->fmt->corder == GSC_CBCR)
>> +                       cfg |= GSC_OUT_YUV422_1P_ORDER_LSB_C;
>> +               if (fmt->corder == GSC_CBCR)
>>                         cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>>                 else
>>                         cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
>> @@ -326,7 +330,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>>                         cfg |= GSC_OUT_YUV420_2P;
>>                 else
>>                         cfg |= GSC_OUT_YUV422_2P;
>> -               if (frame->fmt->corder == GSC_CBCR)
>> +               if (fmt->corder == GSC_CBCR)
>>                         cfg |= GSC_OUT_CHROMA_ORDER_CBCR;
>>                 else
>>                         cfg |= GSC_OUT_CHROMA_ORDER_CRCB;
>> @@ -336,7 +340,7 @@ void gsc_hw_set_out_image_format(struct gsc_ctx *ctx)
>>                 break;
>>         }
>>
>> -       if (is_tiled(frame->fmt))
>> +       if (is_tiled(fmt))
>>                 cfg |= GSC_OUT_TILE_C_16x8 | GSC_OUT_TILE_MODE;
>>
>>  end_set:
>> diff --git a/drivers/media/platform/exynos-gsc/gsc-regs.h b/drivers/media/platform/exynos-gsc/gsc-regs.h
>> index 4678f9a6..b03401dc 100644
>> --- a/drivers/media/platform/exynos-gsc/gsc-regs.h
>> +++ b/drivers/media/platform/exynos-gsc/gsc-regs.h
>> @@ -46,7 +46,7 @@
>>  #define GSC_IN_RGB_SD_WIDE             (0 << 14)
>>  #define GSC_IN_YUV422_1P_ORDER_MASK    (1 << 13)
>>  #define GSC_IN_YUV422_1P_ORDER_LSB_Y   (0 << 13)
>> -#define GSC_IN_YUV422_1P_OEDER_LSB_C   (1 << 13)
>> +#define GSC_IN_YUV422_1P_ORDER_LSB_C   (1 << 13)
>>  #define GSC_IN_CHROMA_ORDER_MASK       (1 << 12)
>>  #define GSC_IN_CHROMA_ORDER_CBCR       (0 << 12)
>>  #define GSC_IN_CHROMA_ORDER_CRCB       (1 << 12)
>> @@ -91,7 +91,7 @@
>>  #define GSC_OUT_RGB_SD_NARROW          (0 << 10)
>>  #define GSC_OUT_YUV422_1P_ORDER_MASK   (1 << 9)
>>  #define GSC_OUT_YUV422_1P_ORDER_LSB_Y  (0 << 9)
>> -#define GSC_OUT_YUV422_1P_OEDER_LSB_C  (1 << 9)
>> +#define GSC_OUT_YUV422_1P_ORDER_LSB_C  (1 << 9)
>>  #define GSC_OUT_CHROMA_ORDER_MASK      (1 << 8)
>>  #define GSC_OUT_CHROMA_ORDER_CBCR      (0 << 8)
>>  #define GSC_OUT_CHROMA_ORDER_CRCB      (1 << 8)
>> --
>> 1.9.0.279.gdc9e3eb
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

[John Sheu]

v4l2-mem2mem presently does not allow VIDIOC_REQBUFS to destroy
outstanding buffers if the queue is of type V4L2_MEMORY_MMAP, and if the
buffers are considered "in use".  This is different behavior than for
other memory types, and prevents us for deallocating buffers in a few
cases:

* In the case that there are outstanding mmap()ed views on the buffer,
  refcounting on the videobuf2 buffer backing the vm_area will track
  lifetime appropriately,
* In the case that the buffer has been exported as a DMABUF, refcounting
  on the videobuf2 bufer backing the DMABUF will track lifetime
  appropriately.

Remove the specific check for type V4L2_MEMOMRY_MMAP when freeing all
buffers through VIDIOC_REQBUFS.

Signed-off-by: John Sheu <sheu@google.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 8e6695c9..5b6f9da6 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -414,8 +414,7 @@ static int __verify_length(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 }
 
 /**
- * __buffer_in_use() - return true if the buffer is in use and
- * the queue cannot be freed (by the means of REQBUFS(0)) call
+ * __buffer_in_use() - return true if the buffer is in use.
  */
 static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
 {
@@ -435,20 +434,6 @@ static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
 }
 
 /**
- * __buffers_in_use() - return true if any buffers on the queue are in use and
- * the queue cannot be freed (by the means of REQBUFS(0)) call
- */
-static bool __buffers_in_use(struct vb2_queue *q)
-{
-	unsigned int buffer;
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
-		if (__buffer_in_use(q, q->bufs[buffer]))
-			return true;
-	}
-	return false;
-}
-
-/**
  * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with information to be
  * returned to userspace
  */
@@ -681,15 +666,6 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	}
 
 	if (req->count == 0 || q->num_buffers != 0 || q->memory != req->memory) {
-		/*
-		 * We already have buffers allocated, so first check if they
-		 * are not in use and can be freed.
-		 */
-		if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
-			dprintk(1, "reqbufs: memory in use, cannot free\n");
-			return -EBUSY;
-		}
-
 		ret = __vb2_queue_free(q, q->num_buffers);
 		if (ret)
 			return ret;
-- 
1.9.0.279.gdc9e3eb

RE: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

[Kamil Debski]

Pawel, Marek,

Before taking this to my tree I wanted to get an ACK from one of the
videobuf2 maintainers. Could you spare a moment to look through this
patch?

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


> -----Original Message-----
> From: John Sheu [mailto:sheu@google.com]
> Sent: Tuesday, March 11, 2014 11:52 PM
> To: linux-media@vger.kernel.org
> Cc: m.chehab@samsung.com; k.debski@samsung.com; posciak@google.com;
> arun.m@samsung.com; kgene.kim@samsung.com; John Sheu
> Subject: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP
> buffers
> 
> v4l2-mem2mem presently does not allow VIDIOC_REQBUFS to destroy
> outstanding buffers if the queue is of type V4L2_MEMORY_MMAP, and if
> the buffers are considered "in use".  This is different behavior than
> for other memory types, and prevents us for deallocating buffers in a
> few
> cases:
> 
> * In the case that there are outstanding mmap()ed views on the buffer,
>   refcounting on the videobuf2 buffer backing the vm_area will track
>   lifetime appropriately,
> * In the case that the buffer has been exported as a DMABUF,
> refcounting
>   on the videobuf2 bufer backing the DMABUF will track lifetime
>   appropriately.
> 
> Remove the specific check for type V4L2_MEMOMRY_MMAP when freeing all
> buffers through VIDIOC_REQBUFS.
> 
> Signed-off-by: John Sheu <sheu@google.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 26 +-----------------------
> --
>  1 file changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c
> index 8e6695c9..5b6f9da6 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -414,8 +414,7 @@ static int __verify_length(struct vb2_buffer *vb,
> const struct v4l2_buffer *b)  }
> 
>  /**
> - * __buffer_in_use() - return true if the buffer is in use and
> - * the queue cannot be freed (by the means of REQBUFS(0)) call
> + * __buffer_in_use() - return true if the buffer is in use.
>   */
>  static bool __buffer_in_use(struct vb2_queue *q, struct vb2_buffer
> *vb)  { @@ -435,20 +434,6 @@ static bool __buffer_in_use(struct
> vb2_queue *q, struct vb2_buffer *vb)  }
> 
>  /**
> - * __buffers_in_use() - return true if any buffers on the queue are in
> use and
> - * the queue cannot be freed (by the means of REQBUFS(0)) call
> - */
> -static bool __buffers_in_use(struct vb2_queue *q) -{
> -	unsigned int buffer;
> -	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
> -		if (__buffer_in_use(q, q->bufs[buffer]))
> -			return true;
> -	}
> -	return false;
> -}
> -
> -/**
>   * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with
> information to be
>   * returned to userspace
>   */
> @@ -681,15 +666,6 @@ static int __reqbufs(struct vb2_queue *q, struct
> v4l2_requestbuffers *req)
>  	}
> 
>  	if (req->count == 0 || q->num_buffers != 0 || q->memory != req-
> >memory) {
> -		/*
> -		 * We already have buffers allocated, so first check if
> they
> -		 * are not in use and can be freed.
> -		 */
> -		if (q->memory == V4L2_MEMORY_MMAP && __buffers_in_use(q)) {
> -			dprintk(1, "reqbufs: memory in use, cannot free\n");
> -			return -EBUSY;
> -		}
> -
>  		ret = __vb2_queue_free(q, q->num_buffers);
>  		if (ret)
>  			return ret;
> --
> 1.9.0.279.gdc9e3eb

Re: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

[Marek Szyprowski]

Hello,

On 2014-04-07 16:41, Kamil Debski wrote:
> Pawel, Marek,
>
> Before taking this to my tree I wanted to get an ACK from one of the
> videobuf2 maintainers. Could you spare a moment to look through this
> patch?

It's not a bug, it is a feature. This was one of the fundamental design 
requirements to allow applications to track if the memory is used or not.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

Le mardi 08 avril 2014 à 12:51 +0200, Marek Szyprowski a écrit :
> Hello,
> 
> On 2014-04-07 16:41, Kamil Debski wrote:
> > Pawel, Marek,
> > 
> > Before taking this to my tree I wanted to get an ACK from one of
> > the
> > videobuf2 maintainers. Could you spare a moment to look through
> > this
> > patch?
> 
> It's not a bug, it is a feature. This was one of the fundamental
> design 
> requirements to allow applications to track if the memory is used or
> not.

I still have the impression it is not fully correct for the case
buffers are exported using DMABUF. Like if the dmabuf was owning the
vb2 buffer instead of the opposite ...

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH 4/4] v4l2-mem2mem: allow reqbufs(0) with "in use" MMAP buffers

[Jean-Michel Hautbois]

Hi,

2015-11-02 19:31 GMT+01:00 Nicolas Dufresne <nicolas.dufresne@collabora.com>:
> Le mardi 08 avril 2014 à 12:51 +0200, Marek Szyprowski a écrit :
>> Hello,
>>
>> On 2014-04-07 16:41, Kamil Debski wrote:
>> > Pawel, Marek,
>> >
>> > Before taking this to my tree I wanted to get an ACK from one of
>> > the
>> > videobuf2 maintainers. Could you spare a moment to look through
>> > this
>> > patch?
>>
>> It's not a bug, it is a feature. This was one of the fundamental
>> design
>> requirements to allow applications to track if the memory is used or
>> not.
>
> I still have the impression it is not fully correct for the case
> buffers are exported using DMABUF. Like if the dmabuf was owning the
> vb2 buffer instead of the opposite ...

I am observing this behaviour too... Tried it, but seems to not do the
job on dmabuf buffers... with gstreamer at least ;-).

JM