Laurent,
thank you for the patch.
It was a bit tricky to get it work as our kernel is based
on 2.6.38, but i succeeded.
The frame_number is incremented now.
The following changes are not clear to me, are they really necessary to
get frame_number incremented?
@@ -350,7 +337,6 @@ static void ccp2_lcx_config(struct isp_ccp2_device
*ccp2,
ISPCCP2_LC01_IRQSTATUS_LC0_CRC_IRQ |
ISPCCP2_LC01_IRQSTATUS_LC0_FSP_IRQ |
ISPCCP2_LC01_IRQSTATUS_LC0_FW_IRQ |
- ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ |
ISPCCP2_LC01_IRQSTATUS_LC0_FSC_IRQ |
ISPCCP2_LC01_IRQSTATUS_LC0_SSC_IRQ;
@@ -378,21 +378,17 @@ static void csi2_timing_config(struct isp_device *isp,
static void csi2_irq_ctx_set(struct isp_device *isp,
struct isp_csi2_device *csi2, int enable)
{
- u32 reg = ISPCSI2_CTX_IRQSTATUS_FE_IRQ;
int i;
- if (csi2->use_fs_irq)
- reg |= ISPCSI2_CTX_IRQSTATUS_FS_IRQ;
-
for (i = 0; i < 8; i++) {
- isp_reg_writel(isp, reg, csi2->regs1,
+ isp_reg_writel(isp, ISPCSI2_CTX_IRQSTATUS_FE_IRQ, csi2->regs1,
ISPCSI2_CTX_IRQSTATUS(i));
if (enable)
isp_reg_set(isp, csi2->regs1, ISPCSI2_CTX_IRQENABLE(i),
- reg);
+ ISPCSI2_CTX_IRQSTATUS_FE_IRQ);
else
isp_reg_clr(isp, csi2->regs1, ISPCSI2_CTX_IRQENABLE(i),
- reg);
+ ISPCSI2_CTX_IRQSTATUS_FE_IRQ);
}
}
Am 09.02.2012 17:08, schrieb Laurent Pinchart:
> Hi Kruno,
>
> On Wednesday 25 January 2012 16:34:26 Kruno Mrak wrote:
>> Hello,
>>
>> we have an omap based intelligent camera and image sensor is connected to
>> camera parallel interface. Image capturing via "CCDC output" works fine.
>> When streaming is on and reading "sequence" variable, it shows always -1.
>> Looking at kernel-source ispvideo.c, i found following if-else statement:
>>
>> /* Do frame number propagation only if this is the output video node.
>> * Frame number either comes from the CSI receivers or it gets
>> * incremented here if H3A is not active.
>> * Note: There is no guarantee that the output buffer will finish
>> * first, so the input number might lag behind by 1 in some cases.
>> */
>> if (video == pipe->output && !pipe->do_propagation)
>> buf->vbuf.sequence = atomic_inc_return(&pipe->frame_number);
>> else
>> buf->vbuf.sequence = atomic_read(&pipe->frame_number);
>>
>> When i change to
>> if (video == pipe->output && pipe->do_propagation)
>> ...
>> the sequence variable is incremented.
>>
>> So my question:
>> Could it be that "pipe->do_propagation" should be tested on true and not on
>> false?
>
> No, the code is correct here. When do_propagation is true, the pipeline's
> frame number is incremented in the frame start interrupt handler. This allows
> synchronization of buffer sequence numbers with the statistics engine frame
> counts.
>
>> If this change is wrong, how can i achieve that the sequence number is
>> incremented?
>
> The driver increments the frame number in the CCP2 and CSI2 receivers
> interrupt handlers. The frame number will thus not be incremented when using
> the CCDC parallel input, which is wrong.
>
> Does the following patch fix your problem ? I haven't been able to test it
> yet with the CCP2 and CSI2 receivers, I might ask you to test a second
> version. Sakari, could you test the patch on the N900 with the CCP2 receiver ?
> I'm currently travelling and haven't brought mine with me.
>
>>From c0d23bfa38fc91f33f39bab9328eda3a4481f152 Mon Sep 17 00:00:00 2001
> From: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Date: Thu, 9 Feb 2012 17:00:45 +0100
> Subject: [PATCH] omap3isp: Fix frame number propagation
>
> When propagating the frame number through the pipeline, the frame number
> must be incremented at frame start by the appropriate IRQ handler. This
> was properly handled for the CSI2 and CCP2 receivers, but not when the
> CCDC parallel interface is used.
>
> ADD frame number incrementation to the HS/VS interrupt handler. As the
> HS/VS interrupt is also generated for frames received by the CSI2 and
> CCP2 receivers, remove explicit propagation handling from the serial
> receivers.
>
> Reported-by: Kruno Mrak <kruno.mrak@xxxxxxxxxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/video/omap3isp/isp.c | 8 --------
> drivers/media/video/omap3isp/ispccdc.c | 3 +++
> drivers/media/video/omap3isp/ispccp2.c | 22 ----------------------
> drivers/media/video/omap3isp/ispcsi2.c | 19 +++----------------
> drivers/media/video/omap3isp/ispcsi2.h | 1 -
> 5 files changed, 6 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c
> index 06afbc1..df6416c 100644
> --- a/drivers/media/video/omap3isp/isp.c
> +++ b/drivers/media/video/omap3isp/isp.c
> @@ -785,14 +785,6 @@ static int isp_pipeline_enable(struct isp_pipeline *pipe,
> }
> }
>
> - /* Frame number propagation. In continuous streaming mode the number
> - * is incremented in the frame start ISR. In mem-to-mem mode
> - * singleshot is used and frame start IRQs are not available.
> - * Thus we have to increment the number here.
> - */
> - if (pipe->do_propagation && mode == ISP_PIPELINE_STREAM_SINGLESHOT)
> - atomic_inc(&pipe->frame_number);
> -
> return 0;
> }
>
> diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
> index eaabc27..8d8d6f3 100644
> --- a/drivers/media/video/omap3isp/ispccdc.c
> +++ b/drivers/media/video/omap3isp/ispccdc.c
> @@ -1410,6 +1410,9 @@ static void ccdc_hs_vs_isr(struct isp_ccdc_device *ccdc)
> struct video_device *vdev = ccdc->subdev.devnode;
> struct v4l2_event event;
>
> + /* Frame number propagation */
> + atomic_inc(&pipe->frame_number);
> +
> memset(&event, 0, sizeof(event));
> event.type = V4L2_EVENT_FRAME_SYNC;
> event.u.frame_sync.frame_sequence = atomic_read(&pipe->frame_number);
> diff --git a/drivers/media/video/omap3isp/ispccp2.c b/drivers/media/video/omap3isp/ispccp2.c
> index 70ddbf3..84014b9 100644
> --- a/drivers/media/video/omap3isp/ispccp2.c
> +++ b/drivers/media/video/omap3isp/ispccp2.c
> @@ -178,19 +178,6 @@ static void ccp2_if_enable(struct isp_ccp2_device *ccp2, u8 enable)
> ISPCCP2_CTRL_MODE | ISPCCP2_CTRL_IF_EN,
> enable ? (ISPCCP2_CTRL_MODE | ISPCCP2_CTRL_IF_EN) : 0);
>
> - /* For frame count propagation */
> - if (pipe->do_propagation) {
> - /* We may want the Frame Start IRQ from LC0 */
> - if (enable)
> - isp_reg_set(isp, OMAP3_ISP_IOMEM_CCP2,
> - ISPCCP2_LC01_IRQENABLE,
> - ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ);
> - else
> - isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCP2,
> - ISPCCP2_LC01_IRQENABLE,
> - ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ);
> - }
> -
> if (!enable && ccp2->vdds_csib)
> regulator_disable(ccp2->vdds_csib);
> }
> @@ -350,7 +337,6 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
> ISPCCP2_LC01_IRQSTATUS_LC0_CRC_IRQ |
> ISPCCP2_LC01_IRQSTATUS_LC0_FSP_IRQ |
> ISPCCP2_LC01_IRQSTATUS_LC0_FW_IRQ |
> - ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ |
> ISPCCP2_LC01_IRQSTATUS_LC0_FSC_IRQ |
> ISPCCP2_LC01_IRQSTATUS_LC0_SSC_IRQ;
>
> @@ -613,14 +599,6 @@ void omap3isp_ccp2_isr(struct isp_ccp2_device *ccp2)
> if (omap3isp_module_sync_is_stopping(&ccp2->wait, &ccp2->stopping))
> return;
>
> - /* Frame number propagation */
> - if (lcx_irqstatus & ISPCCP2_LC01_IRQSTATUS_LC0_FS_IRQ) {
> - struct isp_pipeline *pipe =
> - to_isp_pipeline(&ccp2->subdev.entity);
> - if (pipe->do_propagation)
> - atomic_inc(&pipe->frame_number);
> - }
> -
> /* Handle queued buffers on frame end interrupts */
> if (lcm_irqstatus & ISPCCP2_LCM_IRQSTATUS_EOF_IRQ)
> ccp2_isr_buffer(ccp2);
> diff --git a/drivers/media/video/omap3isp/ispcsi2.c b/drivers/media/video/omap3isp/ispcsi2.c
> index fcb5168..3026215 100644
> --- a/drivers/media/video/omap3isp/ispcsi2.c
> +++ b/drivers/media/video/omap3isp/ispcsi2.c
> @@ -378,21 +378,17 @@ static void csi2_timing_config(struct isp_device *isp,
> static void csi2_irq_ctx_set(struct isp_device *isp,
> struct isp_csi2_device *csi2, int enable)
> {
> - u32 reg = ISPCSI2_CTX_IRQSTATUS_FE_IRQ;
> int i;
>
> - if (csi2->use_fs_irq)
> - reg |= ISPCSI2_CTX_IRQSTATUS_FS_IRQ;
> -
> for (i = 0; i < 8; i++) {
> - isp_reg_writel(isp, reg, csi2->regs1,
> + isp_reg_writel(isp, ISPCSI2_CTX_IRQSTATUS_FE_IRQ, csi2->regs1,
> ISPCSI2_CTX_IRQSTATUS(i));
> if (enable)
> isp_reg_set(isp, csi2->regs1, ISPCSI2_CTX_IRQENABLE(i),
> - reg);
> + ISPCSI2_CTX_IRQSTATUS_FE_IRQ);
> else
> isp_reg_clr(isp, csi2->regs1, ISPCSI2_CTX_IRQENABLE(i),
> - reg);
> + ISPCSI2_CTX_IRQSTATUS_FE_IRQ);
> }
> }
>
> @@ -690,14 +686,6 @@ static void csi2_isr_ctx(struct isp_csi2_device *csi2,
> status = isp_reg_readl(isp, csi2->regs1, ISPCSI2_CTX_IRQSTATUS(n));
> isp_reg_writel(isp, status, csi2->regs1, ISPCSI2_CTX_IRQSTATUS(n));
>
> - /* Propagate frame number */
> - if (status & ISPCSI2_CTX_IRQSTATUS_FS_IRQ) {
> - struct isp_pipeline *pipe =
> - to_isp_pipeline(&csi2->subdev.entity);
> - if (pipe->do_propagation)
> - atomic_inc(&pipe->frame_number);
> - }
> -
> if (!(status & ISPCSI2_CTX_IRQSTATUS_FE_IRQ))
> return;
>
> @@ -1054,7 +1042,6 @@ static int csi2_set_stream(struct v4l2_subdev *sd, int enable)
> case ISP_PIPELINE_STREAM_CONTINUOUS:
> if (omap3isp_csiphy_acquire(csi2->phy) < 0)
> return -ENODEV;
> - csi2->use_fs_irq = pipe->do_propagation;
> if (csi2->output & CSI2_OUTPUT_MEMORY)
> omap3isp_sbl_enable(isp, OMAP3_ISP_SBL_CSI2A_WRITE);
> csi2_configure(csi2);
> diff --git a/drivers/media/video/omap3isp/ispcsi2.h b/drivers/media/video/omap3isp/ispcsi2.h
> index 885ad79..c57729b 100644
> --- a/drivers/media/video/omap3isp/ispcsi2.h
> +++ b/drivers/media/video/omap3isp/ispcsi2.h
> @@ -145,7 +145,6 @@ struct isp_csi2_device {
> u32 output; /* output to CCDC, memory or both? */
> bool dpcm_decompress;
> unsigned int frame_skip;
> - bool use_fs_irq;
>
> struct isp_csiphy *phy;
> struct isp_csi2_ctx_cfg contexts[ISP_CSI2_MAX_CTX_NUM + 1];
>
MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
--
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
