From patchwork Wed Apr 27 11:09:40 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 736151 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter1.kernel.org (8.14.4/8.14.3) with ESMTP id p3RBxxaX004507 for ; Wed, 27 Apr 2011 12:36:18 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758206Ab1D0LJ0 (ORCPT ); Wed, 27 Apr 2011 07:09:26 -0400 Received: from perceval.ideasonboard.com ([95.142.166.194]:51736 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756067Ab1D0LJZ convert rfc822-to-8bit (ORCPT ); Wed, 27 Apr 2011 07:09:25 -0400 Received: from lancelot.localnet (unknown [91.178.135.52]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 358CB35997; Wed, 27 Apr 2011 11:09:24 +0000 (UTC) From: Laurent Pinchart To: Bastian Hecht Subject: Re: OMAP3 ISP deadlocks on my new arm Date: Wed, 27 Apr 2011 13:09:40 +0200 User-Agent: KMail/1.13.6 (Linux/2.6.37-gentoo-r3; KDE/4.6.2; x86_64; ; ) Cc: Sakari Ailus , David Cohen , Linux Media Mailing List References: In-Reply-To: MIME-Version: 1.0 Message-Id: <201104271309.41106.laurent.pinchart@ideasonboard.com> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter1.kernel.org [140.211.167.41]); Wed, 27 Apr 2011 12:36:18 +0000 (UTC) Hi Bastian, On Wednesday 27 April 2011 12:55:24 Bastian Hecht wrote: > 2011/4/27 Bastian Hecht : > > 2011/4/26 Laurent Pinchart : > >> On Tuesday 26 April 2011 17:39:41 Bastian Hecht wrote: > >>> 2011/4/21 Laurent Pinchart : > >>> > On Tuesday 19 April 2011 09:31:05 Sakari Ailus wrote: > >>> >> Laurent Pinchart wrote: > >>> >> ... > >>> >> > >>> >> > That's the ideal situation: sensors should not produce any data > >>> >> > (or rather any transition on the VS/HS signals) when they're > >>> >> > supposed to be stopped. Unfortunately that's not always easy, as > >>> >> > some dumb sensors (or sensor-like hardware) can't be stopped. The > >>> >> > ISP driver should be able to cope with that in a way that doesn't > >>> >> > kill the system completely. > >>> >> > > >>> >> > I've noticed the same issue with a Caspa camera module and an > >>> >> > OMAP3503-based Gumstix. I'll try to come up with a good fix. > >>> >> > >>> >> Hi Laurent, others, > >>> >> > >>> >> Do you think the cause for this is that the system is jammed in > >>> >> handling HS_VS interrupts triggered for every HS? > >>> > > >>> > That was my initial guess, yes. > >>> > > >>> >> A quick fix for this could be just choosing either VS configuration > >>> >> when configuring the CCDC. Alternatively, HS_VS interrupts could be > >>> >> just disabled until omap3isp_configure_interface(). > >>> >> > >>> >> But as the sensor is sending images all the time, proper VS > >>> >> configuration would be needed, or the counting of lines in the CCDC > >>> >> (VD* interrupts) is affected as well. The VD0 interrupt, which is > >>> >> used to trigger an interrupt near the end of the frame, may be > >>> >> triggered one line too early on the first frame, or too late. But > >>> >> this is up to a configuration. I don't think it's a real issue to > >>> >> trigger it one line too early. > >>> >> > >>> >> Anything else? > >>> > >>> Hello Laurent, > >>> > >>> > I've tried delaying the HS_VS interrupt enable to the CCDC > >>> > configuration function, after configuring the bridge (and thus the > >>> > HS/VS interrupt source selection). To my surprise it didn't fix the > >>> > problem, I still get tons of HS_VS interrupts (100000 in about 2.6 > >>> > seconds) that kill the system. > >>> > > >>> > I'll need to hook a scope to the HS and VS signals. > >>> > >>> have you worked on this problem? Today in my setup I took a longer > >>> cable and ran again into the hs/vs interrupt storm (it still works > >>> with a short cable). > >>> I can tackle this issue too, but to avoid double work I wanted to ask > >>> if you worked out something in the meantime. > >> > >> In my case the issue was caused by a combination of two hardware design > >> mistakes. The first one was to use a TXB0801 chip to translate the 3.3V > >> sensor levels to the 1.8V OMAP levels. The TXB0801 4k? output > >> impedance, combined with the OMAP3 100µA pull-ups on the HS and VS > >> signals, produces a ~400mV voltage for low logic levels. > >> > >> Then, the XCLKA signal is next to the VS signal on the cable connecting > >> the camera module to the OMAP board. When XCLKA is turned on, > >> cross-talk produces a 400mV peak-to-peak noise on the VS signal. > >> > >> The combination of those two effects create a noisy VS signal that > >> crosses the OMAP3 input level detection gap at high frequency, leading > >> to an interrupt storm. The workaround is to disable the pull-ups on the > >> HS and VS signals, the solution is to redesign the hardware to replace > >> the level translators and reorganize signals on the camera module > >> cable. > > > > Hi Laurent, > > > >> Is your situation any similar ? > > > > The long data line (~35cm now at 24MHz) certainly can have an impact > > but I haven't measured any crosstalk so far. But I'm on another trail > > now. I found out that on my board the interrupt line is shared with > > 24: 0 INTC omap-iommu.0 > > > > Is the following scenario possible? > > > > 1. The omap-iommu isr is registered > > 2. The isp gets set up (it enables interrupts and disables them again > > at the end of the probe function) > > 3. Later I activate the xclk from within my driver > > 3a. isp_set_xclk() gets the lock omap3isp_get(isp) and so > > enable_interrupts() is called > > 3b. The new xclk on my chip makes my hardware create a hs/vs int > > (either crosstalk, another hardware bug like yours, or simply my chip > > sends a spurious interrupt for any reason) > > 3c. isp_set_xclk() puts the lock omap3isp_put(isp) and so > > disable_interrupts() is called > > > > Can there exist a race condition between the omap3isp raising the > > interrupt pin before 3c or after 3c? > > Argh... I oversaw that the omap3isp isr handler stays registered all > time long so the theory is wrong. No luck :-) The first investigation step is to check which interrupt source causes the interrupts storm. The following test patch should help. Start a capture, wait a couple of settings for ISP interrupts to get disabled, and cat the isr_account sysfs file (/sys/bus/platform/devices/omap3isp/media0/isr_account if I remember correctly). My guess is that you will get approximatively 100000 HS_VS interrupts (31). Let's then move on from there after you've confirmed that the guess is correct. diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c index de2dec5..c4e6455 100644 --- a/drivers/media/video/omap3isp/isp.c +++ b/drivers/media/video/omap3isp/isp.c @@ -400,6 +400,38 @@ static inline void isp_isr_dbg(struct isp_device *isp, u32 irqstatus) printk(KERN_CONT "\n"); } +static unsigned int isp_isr_count[32]; + +static inline void isp_isr_account(struct isp_device *isp, u32 irqstatus) +{ + unsigned int i; + + spin_lock(&isp->isr_account_lock); + for (i = 0; i < 32; i++) { + if (irqstatus & (1 << i)) + isp_isr_count[i]++; + } + spin_unlock(&isp->isr_account_lock); +} + +static ssize_t isp_isr_account_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct isp_device *isp = container_of(to_media_device(to_media_devnode(dev)), struct isp_device, media_dev); + unsigned long flags; + unsigned int i; + int ret = 0; + + spin_lock_irqsave(&isp->isr_account_lock, flags); + for (i = 0; i < 32; i++) + ret += sprintf(buf + ret, "%u\t%u\n", i, isp_isr_count[i]); + spin_unlock_irqrestore(&isp->isr_account_lock, flags); + + return ret; +} + +static DEVICE_ATTR(isr_account, S_IRUGO, isp_isr_account_show, NULL); + static void isp_isr_sbl(struct isp_device *isp) { struct device *dev = isp->dev; @@ -462,6 +494,7 @@ static irqreturn_t isp_isr(int irq, void *_isp) IRQ0STATUS_CCDC_VD0_IRQ | IRQ0STATUS_CCDC_VD1_IRQ | IRQ0STATUS_HS_VS_IRQ; + static unsigned int count = 0; struct isp_device *isp = _isp; u32 irqstatus; int ret; @@ -469,6 +502,10 @@ static irqreturn_t isp_isr(int irq, void *_isp) irqstatus = isp_reg_readl(isp, OMAP3_ISP_IOMEM_MAIN, ISP_IRQ0STATUS); isp_reg_writel(isp, irqstatus, OMAP3_ISP_IOMEM_MAIN, ISP_IRQ0STATUS); + isp_isr_account(isp, irqstatus); + if (count++ >= 100000) + isp_disable_interrupts(isp); + isp_isr_sbl(isp); if (irqstatus & IRQ0STATUS_CSIA_IRQ) { @@ -1971,6 +2008,7 @@ static int isp_remove(struct platform_device *pdev) struct isp_device *isp = platform_get_drvdata(pdev); int i; + device_remove_file(&isp->media_dev.devnode.dev, &dev_attr_isr_account); isp_unregister_entities(isp); isp_cleanup_modules(isp); @@ -2067,6 +2105,7 @@ static int isp_probe(struct platform_device *pdev) mutex_init(&isp->isp_mutex); spin_lock_init(&isp->stat_lock); + spin_lock_init(&isp->isr_account_lock); isp->dev = &pdev->dev; isp->pdata = pdata; @@ -2156,6 +2195,8 @@ static int isp_probe(struct platform_device *pdev) isp_power_settings(isp, 1); omap3isp_put(isp); + ret = device_create_file(&isp->media_dev.devnode.dev, &dev_attr_isr_account); + return 0; error_modules: diff --git a/drivers/media/video/omap3isp/isp.h b/drivers/media/video/omap3isp/isp.h index 2620c40..b3f8448 100644 --- a/drivers/media/video/omap3isp/isp.h +++ b/drivers/media/video/omap3isp/isp.h @@ -259,6 +259,7 @@ struct isp_device { /* ISP Obj */ spinlock_t stat_lock; /* common lock for statistic drivers */ + spinlock_t isr_account_lock; struct mutex isp_mutex; /* For handling ref_count field */ bool needs_reset; int has_context;