2011/4/27 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>:
> Hi Bastian,
>
> On Wednesday 27 April 2011 12:55:24 Bastian Hecht wrote:
>> 2011/4/27 Bastian Hecht <hechtb@xxxxxxxxxxxxxx>:
>> > 2011/4/26 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>:
>> >> On Tuesday 26 April 2011 17:39:41 Bastian Hecht wrote:
>> >>> 2011/4/21 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>:
>> >>> > 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.
>
> 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;
>
> 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.
Hello Laurent,
thank you very much for the patch. It are indeed hs/vs interrupts.
I discovered a heisenbug in my setup :)
When the igep stuck in the interrupts I meassured the hs line. I saw a
slight offset from ground. As I wondered how this small offset can
trigger the interrupt I realized that my system was running again. So
debugging my system with the oscilloscope made it run again. I try to
terminate the hs line with a pulldown to ground and see if I can
"simulate the debugging".
I probably had 24MI/s (megainterrupts per second) cause hs copied the
xclk wave (I discovered it before applying the patch)
Best regards,
Bastian
> --
> Regards,
>
> Laurent Pinchart
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
