Hi. Just have time to test. I apologize for delay. > I'd like to squash all the s3c-camif patches before sending upstream, > if you don't mind. And to add your Signed-off at the final patch. Ok. Squash. > I might have introduced bugs in the image effects handling, hopefully > there is none. I couldn't test it though. Could you test that on your > side with s3c64xx ? Got some error. Seems effect updated only when I set new CrCb value. Seems it's incorrect: case V4L2_CID_COLORFX: if (camif->ctrl_colorfx_cbcr->is_new) { camif->colorfx = camif->ctrl_colorfx->val; /* Set Cb, Cr */ switch (ctrl->val) { case V4L2_COLORFX_SEPIA: camif->ctrl_colorfx_cbcr->val = 0x7391; break; case V4L2_COLORFX_SET_CBCR: /* noop */ break; default: /* for V4L2_COLORFX_BW and others */ camif->ctrl_colorfx_cbcr->val = 0x8080; } } camif->colorfx_cb = camif->ctrl_colorfx_cbcr->val & 0xff; camif->colorfx_cr = camif->ctrl_colorfx_cbcr->val >> 8; break; Moving "camif->colorfx = camif->ctrl_colorfx->val;" out of condition fixes the problem, but setting CrCb value control affect all effects (sepia and BW), not only V4L2_COLORFX_SET_CBCR. Seems condition should be removed and colorfx value should be checked set on every effect change. diff --git a/drivers/media/platform/s3c-camif/camif-capture.c b/drivers/media/platform/s3c-camif/camif-capture.c index ceab03a..9c01f4f 100644 --- a/drivers/media/platform/s3c-camif/camif-capture.c +++ b/drivers/media/platform/s3c-camif/camif-capture.c @@ -1526,19 +1526,17 @@ static int s3c_camif_subdev_s_ctrl(struct v4l2_ctrl *ctrl) switch (ctrl->id) { case V4L2_CID_COLORFX: - if (camif->ctrl_colorfx_cbcr->is_new) { - camif->colorfx = camif->ctrl_colorfx->val; - /* Set Cb, Cr */ - switch (ctrl->val) { - case V4L2_COLORFX_SEPIA: - camif->ctrl_colorfx_cbcr->val = 0x7391; - break; - case V4L2_COLORFX_SET_CBCR: /* noop */ - break; - default: - /* for V4L2_COLORFX_BW and others */ - camif->ctrl_colorfx_cbcr->val = 0x8080; - } + camif->colorfx = camif->ctrl_colorfx->val; + /* Set Cb, Cr */ + switch (ctrl->val) { + case V4L2_COLORFX_SEPIA: + camif->ctrl_colorfx_cbcr->val = 0x7391; + break; + case V4L2_COLORFX_SET_CBCR: /* noop */ + break; + default: + /* for V4L2_COLORFX_BW and others */ + camif->ctrl_colorfx_cbcr->val = 0x8080; } camif->colorfx_cb = camif->ctrl_colorfx_cbcr->val & 0xff; camif->colorfx_cr = camif->ctrl_colorfx_cbcr->val >> 8; With this modification got another issue: set CRCB effect, set CRCB value, set BW effect, set CRCB effect back cause CRCB-value to be reseted to default 0x8080. Is it correct? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html