From patchwork Tue Oct 11 10:29:41 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Enrico X-Patchwork-Id: 8113 Received: from mail.tu-berlin.de ([130.149.7.33]) by www.linuxtv.org with esmtp (Exim 4.72) (envelope-from ) id 1RDZak-0007QE-1X; Tue, 11 Oct 2011 12:29:46 +0200 X-tubIT-Incoming-IP: 209.132.180.67 Received: from vger.kernel.org ([209.132.180.67]) by mail.tu-berlin.de (exim-4.75/mailfrontend-1) with esmtp id 1RDZai-0004Qo-MR; Tue, 11 Oct 2011 12:29:45 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753388Ab1JKK3m (ORCPT + 3 others); Tue, 11 Oct 2011 06:29:42 -0400 Received: from mail-gy0-f174.google.com ([209.85.160.174]:44118 "EHLO mail-gy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753338Ab1JKK3l (ORCPT ); Tue, 11 Oct 2011 06:29:41 -0400 Received: by gyg10 with SMTP id 10so6333548gyg.19 for ; Tue, 11 Oct 2011 03:29:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type; bh=iKQAQvmqhXFTpkMJFAYUZxVF6dcPQdLvuQHc/26c9yc=; b=OhUH2PBzMkeCzJcb7iQttcX8qzBpyiBvwo9K7VLdIA7TiCLtEz1y3r6P9NAMK1vufv sGcH+hc+Dkf2toCRyJ/JsF3LgI9aIwQ/KU0PjPUWzbdCmIUS/DzTEr+1iqYnzt8Mqhby K67tEWWkaa99dnorkog72Rg6W6aInlvL1Jpes= MIME-Version: 1.0 Received: by 10.236.77.104 with SMTP id c68mr28949057yhe.69.1318328981190; Tue, 11 Oct 2011 03:29:41 -0700 (PDT) Received: by 10.236.110.39 with HTTP; Tue, 11 Oct 2011 03:29:41 -0700 (PDT) In-Reply-To: References: <201110081751.38953.laurent.pinchart@ideasonboard.com> Date: Tue, 11 Oct 2011 12:29:41 +0200 X-Google-Sender-Auth: LPd3pX2MoWelms_7p7c23NB9Xec Message-ID: Subject: Re: omap3-isp status From: Enrico To: Javier Martinez Canillas , Laurent Pinchart Cc: Deepthy Ravi , Gary Thomas , Adam Pledger , linux-media@vger.kernel.org, Enric Balletbo i Serra Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-PMX-Version: 5.6.1.2065439, Antispam-Engine: 2.7.2.376379, Antispam-Data: 2011.10.11.101515 X-PMX-Spam: Gauge=IIIIIIII, Probability=8%, Report=' MULTIPLE_RCPTS 0.1, FROM_NAME_ONE_WORD 0.05, MIME_TEXT_ONLY_MP_MIXED 0.05, BODYTEXTP_SIZE_3000_LESS 0, BODY_SIZE_10000_PLUS 0, CTYPE_MULTIPART_NO_QUOTE 0, WEBMAIL_SOURCE 0, __ANY_URI 0, __BOUNCE_CHALLENGE_SUBJ 0, __BOUNCE_NDR_SUBJ_EXEMPT 0, __CP_MEDIA_BODY 0, __CP_URI_IN_BODY 0, __CT 0, __CTYPE_HAS_BOUNDARY 0, __CTYPE_MULTIPART 0, __CTYPE_MULTIPART_MIXED 0, __FRAUD_BODY_WEBMAIL 0, __FRAUD_WEBMAIL 0, __HAS_MSGID 0, __HAS_X_MAILING_LIST 0, __MIME_TEXT_ONLY 0, __MIME_VERSION 0, __MULTIPLE_RCPTS_CC_X2 0, __PHISH_SPEAR_HTTP_RECEIVED 0, __PHISH_SPEAR_STRUCTURE_1 0, __SANE_MSGID 0, __SUBJ_ALPHA_END 0, __TO_MALFORMED_2 0, __URI_NO_WWW 0, __URI_NS ' X-LSpam-Score: 1.1 (+) X-LSpam-Report: No, score=1.1 required=5.0 tests=BAYES_00=-1.9, KB_DATE_CONTAINS_TAB=2.751, RCVD_IN_DNSWL_MED=-2.3, TAB_IN_FROM=2.494, T_DKIM_INVALID=0.01, T_TVD_MIME_EPI=0.01 autolearn=no On Mon, Oct 10, 2011 at 8:18 PM, Javier Martinez Canillas wrote: > On Mon, Oct 10, 2011 at 7:09 PM, Enrico wrote: >> On Mon, Oct 10, 2011 at 6:53 PM, Javier Martinez Canillas >> wrote: >>> On Mon, Oct 10, 2011 at 6:34 PM, Enrico wrote: >>>> Ok, i made it work. It was missing just the config_outlineoffset i >>>> wrote before and a missing FLDMODE in SYNC registers. >>>> >>> >>> Great, do you get the ghosting effect or do you have a clean video? >> >> >> Unfortunately i always get the ghosting effect. But this is something >> we will try to fix later. >> >> > > Agree, we should try to get some code upstream to add interlaced video > and bt.656 support and fix the artifact later. > >>>> Moreover it seems to me that the software-maintained field id >>>> (interlaced_cnt in Javier patches, fldstat in Deepthy patches) is >>>> useless, i've tried to only use the FLDSTAT bit from isp register >>>> (fid) in vd0_isr: >>>> >>>> if (fid == 0) { >>>>     restart = ccdc_isr_buffer(ccdc); >>>>     goto done; >>>> } >>>> >>>> and it works. I've not tested very long frame sequences, only up to 16 >>>> frames. The only issue is that the first frame could be half-green >>>> because a field is missing. >>>> >>> >>> Yes, when I tried Deepthy patches I realized that the fldstat was not >>> in sync with the frames, but probably I made something wrong. >> >> >> I had noticed the same thing, but now i tested it and it is ok, maybe >> my fault too. >> >> >>> We had the same problem with the hal-green frame. Our solution was to >>> synchronize the CCDC with the first even field looking at fdstat on >>> the VD1 interrupt handler and forcing to start processing from an ODD >>> sub-frame. >> >> Thinking more about it, it's ugly to have that half-green video frame >> even if it's just one. It's better to keep your or Deepthy solution. >> >> Enrico >> > > Well, that is something that can be fixed later also. Can you send to > the list your patches? So, Laurent, Sakari and others than know more > about the ISP can review it. I hope they can find the cause for the > artifact. I'm attaching some fixes (taken from Deepthy patches) to be applied on top of your v2 patches, with those i can grab frames but i only get garbage. I think the problem is that it always hits this in ccdc_isr_buffer: if (ccdc_sbl_wait_idle(ccdc, 1000)) { dev_info(isp->dev, "CCDC won't become idle!\n"); goto done; } so the video buffer never gets updated. At this point i think it is better to go on with my port of Deepthy patches and try to solve the ghosting issues, maybe with your fixes about buffer decoupling. Laurent, what do you suggest to do? Enrico From 078f2843ba94cfbc150f6c01cc614c0ed3a35fd4 Mon Sep 17 00:00:00 2001 From: Enrico Butera Date: Tue, 11 Oct 2011 11:51:38 +0200 Subject: [PATCH] omap3isp: some fixes for Javier v2 patches Signed-off-by: Enrico Butera --- drivers/media/video/omap3isp/ispccdc.c | 41 +++++++++++++++++++++++-------- 1 files changed, 30 insertions(+), 11 deletions(-) diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c index f1da49c..0cb4e36 100644 --- a/drivers/media/video/omap3isp/ispccdc.c +++ b/drivers/media/video/omap3isp/ispccdc.c @@ -43,6 +43,7 @@ __ccdc_get_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh, unsigned int pad, enum v4l2_subdev_format_whence which); static bool ccdc_input_is_bt656(struct isp_ccdc_device *ccdc); static bool ccdc_input_is_fldmode(struct isp_ccdc_device *ccdc); +static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event); static const unsigned int ccdc_fmts[] = { V4L2_MBUS_FMT_Y8_1X8, @@ -795,11 +796,17 @@ static void ccdc_apply_controls(struct isp_ccdc_device *ccdc) void omap3isp_ccdc_restore_context(struct isp_device *isp) { struct isp_ccdc_device *ccdc = &isp->isp_ccdc; + struct v4l2_mbus_framefmt *format; isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG, ISPCCDC_CFG_VDLC); - ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF - | OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP; + /* CCDC_PAD_SINK */ + format = &ccdc->formats[CCDC_PAD_SINK]; + if ((format->code != V4L2_MBUS_FMT_UYVY8_2X8) && + (format->code != V4L2_MBUS_FMT_UYVY8_2X8)) + ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF + | OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP; + ccdc_apply_controls(ccdc); ccdc_configure_fpc(ccdc); } @@ -1021,10 +1028,10 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc, if (pdata && pdata->bt656) isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF, - ISPCCDC_REC656IF_R656ON); + ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH); else isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF, - ISPCCDC_REC656IF_R656ON); + ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH); } /* CCDC formats descriptions */ @@ -1187,7 +1194,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) ccdc_pattern = ccdc_sgrbg_pattern; break; } - ccdc_config_imgattr(ccdc, ccdc_pattern); + if ((format->code != V4L2_MBUS_FMT_YUYV8_2X8) && + (format->code != V4L2_MBUS_FMT_UYVY8_2X8)) + ccdc_config_imgattr(ccdc, ccdc_pattern); /* In BT.656 a pixel is representd using two bytes */ if (pdata->bt656) @@ -1219,7 +1228,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO); isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV0_SHIFT, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START); - isp_reg_writel(isp, nlv << ISPCCDC_VERT_LINES_NLV_SHIFT, + isp_reg_writel(isp, (((format->height >> 1) - 1) /*nlv*/ << ISPCCDC_VERT_LINES_NLV_SHIFT), OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES); isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV1_SHIFT, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START); @@ -1278,6 +1287,11 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc) unlock: spin_unlock_irqrestore(&ccdc->lsc.req_lock, flags); + if (pdata->bt656) + ccdc->update = OMAP3ISP_CCDC_BLCLAMP; + else + ccdc->update = 0; + ccdc_apply_controls(ccdc); } @@ -1299,6 +1313,11 @@ static int ccdc_disable(struct isp_ccdc_device *ccdc) ccdc->stopping = CCDC_STOP_REQUEST; spin_unlock_irqrestore(&ccdc->lock, flags); + __ccdc_lsc_enable(ccdc, 0); + __ccdc_enable(ccdc, 0); + ccdc->stopping = CCDC_STOP_EXECUTED; + __ccdc_handle_stopping(ccdc, CCDC_STOP_FINISHED); + ret = wait_event_timeout(ccdc->wait, ccdc->stopping == CCDC_STOP_FINISHED, msecs_to_jiffies(2000)); @@ -1522,7 +1541,7 @@ static int ccdc_isr_buffer(struct isp_ccdc_device *ccdc) /* In interlaced mode a frame is composed of two subframes so we don't have * to change the CCDC output memory on every end of frame. */ - if (!ccdc_input_is_fldmode(ccdc)) { + if (ccdc_input_is_fldmode(ccdc)) { if (!ccdc->interlaced_cnt) { ccdc->interlaced_cnt = 1; restart = 1; @@ -1656,7 +1675,7 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events) if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED) return 0; - if (events & IRQ0STATUS_CCDC_VD1_IRQ) + if ((events & IRQ0STATUS_CCDC_VD1_IRQ) && !ccdc_input_is_bt656(ccdc)) ccdc_vd1_isr(ccdc); ccdc_lsc_isr(ccdc, events); @@ -1664,7 +1683,7 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events) if (events & IRQ0STATUS_CCDC_VD0_IRQ) ccdc_vd0_isr(ccdc); - if (events & IRQ0STATUS_HS_VS_IRQ) + if ((events & IRQ0STATUS_HS_VS_IRQ) && !ccdc_input_is_bt656(ccdc)) ccdc_hs_vs_isr(ccdc); return 0; @@ -1774,7 +1793,7 @@ static int ccdc_set_stream(struct v4l2_subdev *sd, int enable) * links are inactive. */ ccdc_config_vp(ccdc); - ccdc_enable_vp(ccdc, 1); + ccdc_enable_vp(ccdc, 0); ccdc->error = 0; ccdc_print_status(ccdc); } @@ -2344,7 +2363,7 @@ int omap3isp_ccdc_init(struct isp_device *isp) ccdc->vpcfg.pixelclk = 0; - ccdc->update = OMAP3ISP_CCDC_BLCLAMP; + ccdc->update = 0; ccdc_apply_controls(ccdc); ret = ccdc_init_entities(ccdc); -- 1.7.4.1