diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index ca31c45..046ebf6 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -81,6 +81,9 @@ static int s3c_camif_hw_init(struct camif_dev *camif, 
struct camif_vp *vp)
  	camif_hw_set_source_format(camif);
  	camif_hw_set_camera_crop(camif);
  	camif_hw_set_test_pattern(camif, camif->test_pattern->val);
+	if (ip_rev >= S3C2450_CAMIF_IP_REV)
+		camif_hw_set_effect(camif, camif->effect->val,
+				camif->effect_cr->val, camif->effect_cb->val);
  	if (ip_rev == S3C6410_CAMIF_IP_REV)
  		camif_hw_set_input_path(vp);
  	camif_cfg_video_path(vp);
@@ -108,8 +111,8 @@ static int s3c_camif_hw_vp_init(struct camif_dev 
*camif, struct camif_vp *vp)
  	if (ip_rev == S3C244X_CAMIF_IP_REV)
  		camif_hw_clear_fifo_overflow(vp);
  	camif_cfg_video_path(vp);
-	if (ip_rev == S3C6410_CAMIF_IP_REV)
-		camif_hw_set_effect(vp, false);
+	if (ip_rev >= S3C2450_CAMIF_IP_REV)
+		camif_hw_set_effect(camif, 0, 0, 0);
  	vp->state &= ~ST_VP_CONFIG;

  	spin_unlock_irqrestore(&camif->slock, flags);
@@ -374,6 +377,10 @@ irqreturn_t s3c_camif_irq_handler(int irq, void *priv)
  		camif_hw_set_scaler(vp);
  		camif_hw_set_flip(vp);
  		camif_hw_set_test_pattern(camif, camif->test_pattern->val);
+		if (ip_rev >= S3C2450_CAMIF_IP_REV)
+			camif_hw_set_effect(camif, camif->effect->val,
+					camif->effect_cr->val,
+					camif->effect_cb->val);
  		vp->state &= ~ST_VP_CONFIG;
  	}
  unlock:
@@ -1530,7 +1537,7 @@ static const struct v4l2_ctrl_ops 
s3c_camif_subdev_ctrl_ops = {
  	.s_ctrl	= s3c_camif_subdev_s_ctrl,
  };

-static const struct v4l2_ctrl_config s3c_camif_priv_ctrl = {
+static const struct v4l2_ctrl_config s3c_camif_priv_ctrl_test = {
  	.ops	= &s3c_camif_subdev_ctrl_ops,
  	.id	= V4L2_CTRL_CLASS_USER | 0x1001,
  	.type	= V4L2_CTRL_TYPE_INTEGER,
@@ -1541,6 +1548,39 @@ static const struct v4l2_ctrl_config 
s3c_camif_priv_ctrl = {
  	.def	= 0,
  };

+static const struct v4l2_ctrl_config s3c_camif_priv_ctrl_eff = {
+	.ops	= &s3c_camif_subdev_ctrl_ops,
+	.id	= V4L2_CTRL_CLASS_USER | 0x1002,
+	.type	= V4L2_CTRL_TYPE_INTEGER,
+	.name	= "Effect",
+	.min	= 0,
+	.max	= 5,
+	.step	= 1,
+	.def	= 0,
+};
+
+static const struct v4l2_ctrl_config s3c_camif_priv_ctrl_eff_cb = {
+	.ops	= &s3c_camif_subdev_ctrl_ops,
+	.id	= V4L2_CTRL_CLASS_USER | 0x1003,
+	.type	= V4L2_CTRL_TYPE_INTEGER,
+	.name	= "PAT_Cb",
+	.min	= 16,
+	.max	= 240,
+	.step	= 1,
+	.def	= 128,
+};
+
+static const struct v4l2_ctrl_config s3c_camif_priv_ctrl_eff_cr = {
+	.ops	= &s3c_camif_subdev_ctrl_ops,
+	.id	= V4L2_CTRL_CLASS_USER | 0x1004,
+	.type	= V4L2_CTRL_TYPE_INTEGER,
+	.name	= "PAT_Cr",
+	.min	= 16,
+	.max	= 240,
+	.step	= 1,
+	.def	= 128,
+};

SN:
There is no need to create these 3 private controls, we have now standard
controls for this image effect. Can you rework it to use V4L2_CID_COLORFX
and V4L2_CID_COLORFX_CBCR controls ? Not used option of V4L2_CID_COLORFX
can be easily masked off by passing proper mask to v4l2_ctrl_new_std().
Unfortunately the CB/CR coefficients will have fixed min and max values
then - 0, 255. That shouldn't be a big issue though.

AFAICT the range for CB/CR depends on the CSCR2Y_c bit which means:

"YCbCr Data Dynamic Range Selection for the Color Space Conversion RGB
to YCbCr"

CSCR2Y_c	1 : Wide => Y/Cb/Cr (0 ~ 255) : Wide default
		0 : Narrow => Y (16 ~ 235), Cb/Cr (16 ~ 240)

By default CSCR2Y_c bit is set so we get 0 ~ 255 range.


  int s3c_camif_create_subdev(struct camif_dev *camif)
  {
  	struct v4l2_ctrl_handler *handler = &camif->ctrl_handler;
@@ -1560,10 +1600,18 @@ int s3c_camif_create_subdev(struct camif_dev *camif)
  	if (ret)
  		return ret;

-	v4l2_ctrl_handler_init(handler, 1);
+	v4l2_ctrl_handler_init(handler, 4);
  	camif->test_pattern = v4l2_ctrl_new_custom(handler,
-					&s3c_camif_priv_ctrl, NULL);
+					&s3c_camif_priv_ctrl_test, NULL);
+	camif->effect = v4l2_ctrl_new_custom(handler,
+					&s3c_camif_priv_ctrl_eff, NULL);
+	camif->effect_cr = v4l2_ctrl_new_custom(handler,
+					&s3c_camif_priv_ctrl_eff_cb, NULL);
+	camif->effect_cb = v4l2_ctrl_new_custom(handlerEffect,
+					&s3c_camif_priv_ctrl_eff_cr, NULL);
+
  	if (handler->error) {
+		v4l2_ctrl_handler_free(handler);
  		media_entity_cleanup(&sd->entity);
  		return handler->error;
  	}
diff --git a/drivers/media/platform/s3c-camif/camif-core.h 
b/drivers/media/platform/s3c-camif/camif-core.h
index 96f5d3d..5f9eb3a 100644
--- a/drivers/media/platform/s3c-camif/camif-core.h
+++ b/drivers/media/platform/s3c-camif/camif-core.h
@@ -39,6 +39,8 @@
  #define CAMIF_STOP_TIMEOUT	1500 /* ms */

  #define S3C244X_CAMIF_IP_REV	0x20 /* 2.0 */
+#define S3C2450_CAMIF_IP_REV	0x30 /* 3.0 - not implemented, not tested */
+#define S3C6400_CAMIF_IP_REV	0x31 /* 3.1 - not implemented, not tested */
  #define S3C6410_CAMIF_IP_REV	0x32 /* 3.2 */

  /* struct camif_vp::state */
@@ -277,6 +279,9 @@ struct camif_dev {

  	struct v4l2_ctrl_handler	ctrl_handler;
  	struct v4l2_ctrl		*test_pattern;
+	struct v4l2_ctrl		*effect;
+	struct v4l2_ctrl		*effect_cb;
+	struct v4l2_ctrl		*effect_cr;

As explained above 2 controls can be used instead.

  	struct camif_vp			vp[CAMIF_VP_NUM];
  	struct vb2_alloc_ctx		*alloc_ctx;
diff --git a/drivers/media/platform/s3c-camif/camif-regs.c 
b/drivers/media/platform/s3c-camif/camif-regs.c
index d8c55dc..1b03f73 100644
--- a/drivers/media/platform/s3c-camif/camif-regs.c
+++ b/drivers/media/platform/s3c-camif/camif-regs.c
@@ -57,6 +57,33 @@ void camif_hw_set_test_pattern(struct camif_dev 
*camif, unsigned int pattern)
  	camif_write(camif, S3C_CAMIF_REG_CIGCTRL, cfg);
  }

+void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
+			unsigned int cr, unsigned int cb)
+{
+	u32 cfg;
+
+	if (camif->variant->ip_revision < S3C2450_CAMIF_IP_REV)
+		return;

I think you can remove this check. Since camif_hw_set_effect() is always
called conditionally.

+
+	cfg = camif_read(camif, S3C_CAMIF_REG_CIIMGEFF(camif->vp->offset));
+	/* Set effect */
+	cfg &= ~CIIMGEFF_FIN_MASK;
+	cfg |= (effect << 26);
+	/* Set both paths */
+	if (camif->variant->ip_revision >= S3C6400_CAMIF_IP_REV) {
+		if (effect)
+			cfg |= CIIMGEFF_IE_ENABLE_MASK;
+		else
+			cfg &= ~CIIMGEFF_IE_ENABLE_MASK;
+	}
+	/* Set Cr, Cb */
+	if (effect == CIIMGEFF_FIN_ARBITRARY) {
+		cfg &= ~CIIMGEFF_PAT_CBCR_MASK;
+		cfg |= ((cb & 0xFF) << 13) | (cr & 0xFF);

Can you please use lower case for hex numbers ?

+	}
+	camif_write(camif, S3C_CAMIF_REG_CIIMGEFF(camif->vp->offset), cfg);
+}
+
  static const u32 src_pixfmt_map[8][2] = {
  	{ V4L2_MBUS_FMT_YUYV8_2X8, CISRCFMT_ORDER422_YCBYCR },
  	{ V4L2_MBUS_FMT_YVYU8_2X8, CISRCFMT_ORDER422_YCRYCB },
@@ -473,17 +500,6 @@ void camif_hw_set_lastirq(struct camif_vp *vp, int 
enable)
  	camif_write(vp->camif, addr, cfg);
  }

-void camif_hw_set_effect(struct camif_vp *vp, bool active)
-{
-	u32 cfg = 0;
-
-	if (active) {
-		/* TODO: effects support on 64xx */
-	}
-
-	camif_write(vp->camif, S3C_CAMIF_REG_CIIMGEFF, cfg);
-}
-
  void camif_hw_enable_capture(struct camif_vp *vp)
  {
  	struct camif_dev *camif = vp->camif;
diff --git a/drivers/media/platform/s3c-camif/camif-regs.h 
b/drivers/media/platform/s3c-camif/camif-regs.h
index a3488ca..213adbc 100644
--- a/drivers/media/platform/s3c-camif/camif-regs.h
+++ b/drivers/media/platform/s3c-camif/camif-regs.h
@@ -177,8 +177,9 @@
  #define S3C_CAMIF_REG_CICPTSEQ			0xc4

  /* Image effects */
-#define S3C_CAMIF_REG_CIIMGEFF			0xd0
+#define S3C_CAMIF_REG_CIIMGEFF(_offs)		(0xb0 + (_offs))
  #define  CIIMGEFF_IE_ENABLE(id)			(1 << (30 + (id)))
+#define  CIIMGEFF_IE_ENABLE_MASK		(3 << 30)
  /* Image effect: 1 - after scaler, 0 - before scaler */
  #define  CIIMGEFF_IE_AFTER_SC			(1 << 29)
  #define  CIIMGEFF_FIN_MASK			(7 << 26)
@@ -243,7 +244,6 @@ void camif_hw_clear_fifo_overflow(struct camif_vp *vp);
  void camif_hw_set_lastirq(struct camif_vp *vp, int enable);
  void camif_hw_set_input_path(struct camif_vp *vp);
  void camif_hw_enable_scaler(struct camif_vp *vp, bool on);
-void camif_hw_set_effect(struct camif_vp *vp, bool active);
  void camif_hw_enable_capture(struct camif_vp *vp);
  void camif_hw_disable_capture(struct camif_vp *vp);
  void camif_hw_set_camera_bus(struct camif_dev *camif);
@@ -254,6 +254,8 @@ void camif_hw_set_flip(struct camif_vp *vp);
  void camif_hw_set_output_dma(struct camif_vp *vp);
  void camif_hw_set_target_format(struct camif_vp *vp);
  void camif_hw_set_test_pattern(struct camif_dev *camif, unsigned int 
pattern);
+void camif_hw_set_effect(struct camif_dev *camif, unsigned int effect,
+			unsigned int cr, unsigned int cb);
  void camif_hw_set_output_addr(struct camif_vp *vp, struct camif_addr 
*paddr,
