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-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to