[PATCH 0/2] Fix issues with frame reception from CSI on i.MX31

[PATCH 0/2] Fix issues with frame reception from CSI on i.MX31

[Anatolij Gustschin]

On some camera systems we do not tolerate the losing of
captured frames. We observed losing of the first frame
from CSI when double buffering is used (multiple buffers
queued by the mx3-camera driver).

The patches provide fixes for the observed problem.

Anatolij Gustschin (2):
  v4l: soc-camera: start stream after queueing the buffers
  dma: ipu_idmac: do not lose valid received data in the irq handler

 drivers/dma/ipu/ipu_idmac.c      |   50 --------------------------------------
 drivers/media/video/soc_camera.c |    4 +-
 2 files changed, 2 insertions(+), 52 deletions(-)

[PATCH 1/2] v4l: soc-camera: start stream after queueing the buffers

[Anatolij Gustschin]

Some camera systems have strong requirement for capturing
an exact number of frames after starting the stream and do
not tolerate losing captured frames. By starting the stream
after the videobuf has queued the buffers, we ensure that
no frame will be lost.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/media/video/soc_camera.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index a66811b..7299de0 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -646,11 +646,11 @@ static int soc_camera_streamon(struct file *file, void *priv,
 	if (icd->streamer != file)
 		return -EBUSY;
 
-	v4l2_subdev_call(sd, video, s_stream, 1);
-
 	/* This calls buf_queue from host driver's videobuf_queue_ops */
 	ret = videobuf_streamon(&icd->vb_vidq);
 
+	v4l2_subdev_call(sd, video, s_stream, 1);
+
 	return ret;
 }
 
-- 
1.7.1

[PATCH 1/2] v4l: soc-camera: start stream after queueing the buffers

[Guennadi Liakhovetski]

On Wed, 26 Jan 2011, Anatolij Gustschin wrote:

> Some camera systems have strong requirement for capturing
> an exact number of frames after starting the stream and do
> not tolerate losing captured frames. By starting the stream
> after the videobuf has queued the buffers, we ensure that
> no frame will be lost.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
>  drivers/media/video/soc_camera.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
> index a66811b..7299de0 100644
> --- a/drivers/media/video/soc_camera.c
> +++ b/drivers/media/video/soc_camera.c
> @@ -646,11 +646,11 @@ static int soc_camera_streamon(struct file *file, void *priv,
>  	if (icd->streamer != file)
>  		return -EBUSY;
>  
> -	v4l2_subdev_call(sd, video, s_stream, 1);
> -
>  	/* This calls buf_queue from host driver's videobuf_queue_ops */
>  	ret = videobuf_streamon(&icd->vb_vidq);
>  
> +	v4l2_subdev_call(sd, video, s_stream, 1);
> +

After a bit more testing I'll make this to

+	if (!ret)
+		v4l2_subdev_call(sd, video, s_stream, 1);
+

Ok? Or you can submit a v2 yourself, if you like - when you fix the 
comment in the other patch from this series.

>  	return ret;
>  }
>  
> -- 
> 1.7.1

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 1/2] v4l: soc-camera: start stream after queueing the buffers

[Anatolij Gustschin]

On Sat, 29 Jan 2011 20:16:42 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
...
> > --- a/drivers/media/video/soc_camera.c
> > +++ b/drivers/media/video/soc_camera.c
> > @@ -646,11 +646,11 @@ static int soc_camera_streamon(struct file *file, void *priv,
> >  	if (icd->streamer != file)
> >  		return -EBUSY;
> >  
> > -	v4l2_subdev_call(sd, video, s_stream, 1);
> > -
> >  	/* This calls buf_queue from host driver's videobuf_queue_ops */
> >  	ret = videobuf_streamon(&icd->vb_vidq);
> >  
> > +	v4l2_subdev_call(sd, video, s_stream, 1);
> > +
> 
> After a bit more testing I'll make this to
> 
> +	if (!ret)
> +		v4l2_subdev_call(sd, video, s_stream, 1);
> +
> 
> Ok? Or you can submit a v2 yourself, if you like - when you fix the 
> comment in the other patch from this series.

I'll submit a v2 patch since I have to resubmit the other patch, too.


Thanks,
Anatolij

[PATCH 1/2 v2] v4l: soc-camera: start stream after queueing the buffers

[Anatolij Gustschin]

Some camera systems have strong requirement for capturing
an exact number of frames after starting the stream and do
not tolerate losing captured frames. By starting the stream
after the videobuf has queued the buffers, we ensure that
no frame will be lost.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
    Check for return value of videobuf_streamon() before
    starting the stream, as suggested by Guennadi.

 drivers/media/video/soc_camera.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index a66811b..e09bec0 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -646,11 +646,12 @@ static int soc_camera_streamon(struct file *file, void *priv,
 	if (icd->streamer != file)
 		return -EBUSY;
 
-	v4l2_subdev_call(sd, video, s_stream, 1);
-
 	/* This calls buf_queue from host driver's videobuf_queue_ops */
 	ret = videobuf_streamon(&icd->vb_vidq);
 
+	if (!ret)
+		v4l2_subdev_call(sd, video, s_stream, 1);
+
 	return ret;
 }
 
-- 
1.7.1

[PATCH 2/2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

Currently when two or more buffers are queued by the camera driver
and so the double buffering is enabled in the idmac, we lose the
first frame comming from CSI since it is just dropped by the
interrupt handler. The reason for this dropping is that the interrupt
handler misleadingly assumes that the cur_buf flag is pointing to the
buffer used by the IDMAC. Actually it is not the case since the
CUR_BUF flag will be flipped by the FSU when the FSU is sending the
<TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
When sending this singal, FSU updates the DMA_CUR_BUF and the
DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
is cleared, indicating that the frame data is beeing written by
the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
set to the ready state again by the MCU, when it has handled the
received data. DMA_BUFx_RDY won't be set by the IPU, so waiting
for this event in the interrupt handler is wrong. Actually there
is no spurious interrupt as described in the comments, this
is the valid DMAIC_7_EOF interrupt indicating reception of the
frame from CSI.

This has been verified on the hardware which is triggering the
image sensor by the programmable state machine, allowing to
obtain exact number of frames. On this hardware we do not tolerate
losing frames.

This patch also removes resetting the DMA_BUFx_RDY flags of
all channels in ipu_disable_channel() since transfers on other
DMA channels might be triggered by other running tasks and the
buffers should always be ready for data sending or reception.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/dma/ipu/ipu_idmac.c |   50 -------------------------------------------
 1 files changed, 0 insertions(+), 50 deletions(-)

diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index cb26ee9..c1a125e 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac, struct idmac_channel *ichan,
 	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
 	idmac_write_icreg(ipu, reg & ~chan_mask, IDMAC_CHA_EN);
 
-	/*
-	 * Problem (observed with channel DMAIC_7): after enabling the channel
-	 * and initialising buffers, there comes an interrupt with current still
-	 * pointing at buffer 0, whereas it should use buffer 0 first and only
-	 * generate an interrupt when it is done, then current should already
-	 * point to buffer 1. This spurious interrupt also comes on channel
-	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
-	 * first interrupt, there comes the second with current correctly
-	 * pointing to buffer 1 this time. But sometimes this second interrupt
-	 * doesn't come and the channel hangs. Clearing BUFx_RDY when disabling
-	 * the channel seems to prevent the channel from hanging, but it doesn't
-	 * prevent the spurious interrupt. This might also be unsafe. Think
-	 * about the IDMAC controller trying to switch to a buffer, when we
-	 * clear the ready bit, and re-enable it a moment later.
-	 */
-	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
-	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
-	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF0_RDY);
-
-	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
-	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
-	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF1_RDY);
-
 	spin_unlock_irqrestore(&ipu->lock, flags);
 
 	return 0;
@@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
 
 	/* Other interrupts do not interfere with this channel */
 	spin_lock(&ichan->lock);
-	if (unlikely(chan_id != IDMAC_SDC_0 && chan_id != IDMAC_SDC_1 &&
-		     ((curbuf >> chan_id) & 1) == ichan->active_buffer &&
-		     !list_is_last(ichan->queue.next, &ichan->queue))) {
-		int i = 100;
-
-		/* This doesn't help. See comment in ipu_disable_channel() */
-		while (--i) {
-			curbuf = idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
-			if (((curbuf >> chan_id) & 1) != ichan->active_buffer)
-				break;
-			cpu_relax();
-		}
-
-		if (!i) {
-			spin_unlock(&ichan->lock);
-			dev_dbg(dev,
-				"IRQ on active buffer on channel %x, active "
-				"%d, ready %x, %x, current %x!\n", chan_id,
-				ichan->active_buffer, ready0, ready1, curbuf);
-			return IRQ_NONE;
-		} else
-			dev_dbg(dev,
-				"Buffer deactivated on channel %x, active "
-				"%d, ready %x, %x, current %x, rest %d!\n", chan_id,
-				ichan->active_buffer, ready0, ready1, curbuf, i);
-	}
-
 	if (unlikely((ichan->active_buffer && (ready1 >> chan_id) & 1) ||
 		     (!ichan->active_buffer && (ready0 >> chan_id) & 1)
 		     )) {
-- 
1.7.1

[PATCH 2/2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

Reading the commit message again I now realize that there is
a mistake. 

On Wed, 26 Jan 2011 09:49:49 +0100
Anatolij Gustschin <agust@denx.de> wrote:
...
> received data. DMA_BUFx_RDY won't be set by the IPU, so waiting
> for this event in the interrupt handler is wrong.

This should read 'DMAIC_x_CUR_BUF flag won't be flipped here by
the IPU, so waiting for this event in the EOF interrupt handler
is wrong.'

I'll submit another patch fixing the commit message.

Anatolij

[PATCH 2/2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Dan Williams]

On Thu, Jan 27, 2011 at 12:22 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Reading the commit message again I now realize that there is
> a mistake.
>
> On Wed, 26 Jan 2011 09:49:49 +0100
> Anatolij Gustschin <agust@denx.de> wrote:
> ...
>> received data. DMA_BUFx_RDY won't be set by the IPU, so waiting
>> for this event in the interrupt handler is wrong.
>
> This should read 'DMAIC_x_CUR_BUF flag won't be flipped here by
> the IPU, so waiting for this event in the EOF interrupt handler
> is wrong.'
>
> I'll submit another patch fixing the commit message.
>

Ok, also looking for a reviewed/acked by Guennadi.

--
Dan

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

Currently when two or more buffers are queued by the camera driver
and so the double buffering is enabled in the idmac, we lose one
frame comming from CSI since the reporting of arrival of the first
frame is deferred by the DMAIC_7_EOF interrupt handler and reporting
of the arrival of the last frame is not done at all. So when requesting
N frames from the image sensor we actually receive N - 1 frames in
user space.

The reason for this behaviour is that the DMAIC_7_EOF interrupt
handler misleadingly assumes that the CUR_BUF flag is pointing to the
buffer used by the IDMAC. Actually it is not the case since the
CUR_BUF flag will be flipped by the FSU when the FSU is sending the
<TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
When sending this singal, FSU updates the DMA_CUR_BUF and the
DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
is cleared, indicating that the frame data is beeing written by
the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
set to the ready state again by the MCU, when it has handled the
received data. DMAIC_7_CUR_BUF flag won't be flipped here by the
IPU, so waiting for this event in the EOF interrupt handler is wrong.
Actually there is no spurious interrupt as described in the comments,
this is the valid DMAIC_7_EOF interrupt indicating reception of the
frame from CSI.

The patch removes code that waits for flipping of the DMAIC_7_CUR_BUF
flag in the DMAIC_7_EOF interrupt handler. As the comment in the
current code denotes, this waiting doesn't help anyway. As a result
of this removal the reporting of the first arrived frame is not
deferred to the time of arrival of the next frame and the drivers
software flag 'ichan->active_buffer' is in sync with DMAIC_7_CUR_BUF
flag, so the reception of all requested frames works.

This has been verified on the hardware which is triggering the
image sensor by the programmable state machine, allowing to
obtain exact number of frames. On this hardware we do not tolerate
losing frames.

This patch also removes resetting the DMA_BUFx_RDY flags of
all channels in ipu_disable_channel() since transfers on other
DMA channels might be triggered by other running tasks and the
buffers should always be ready for data sending or reception.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
    Revise the commit message to provide more and correct
    information about the observed problem and proposed fix

 drivers/dma/ipu/ipu_idmac.c |   50 -------------------------------------------
 1 files changed, 0 insertions(+), 50 deletions(-)

diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index cb26ee9..c1a125e 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac, struct idmac_channel *ichan,
 	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
 	idmac_write_icreg(ipu, reg & ~chan_mask, IDMAC_CHA_EN);
 
-	/*
-	 * Problem (observed with channel DMAIC_7): after enabling the channel
-	 * and initialising buffers, there comes an interrupt with current still
-	 * pointing at buffer 0, whereas it should use buffer 0 first and only
-	 * generate an interrupt when it is done, then current should already
-	 * point to buffer 1. This spurious interrupt also comes on channel
-	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
-	 * first interrupt, there comes the second with current correctly
-	 * pointing to buffer 1 this time. But sometimes this second interrupt
-	 * doesn't come and the channel hangs. Clearing BUFx_RDY when disabling
-	 * the channel seems to prevent the channel from hanging, but it doesn't
-	 * prevent the spurious interrupt. This might also be unsafe. Think
-	 * about the IDMAC controller trying to switch to a buffer, when we
-	 * clear the ready bit, and re-enable it a moment later.
-	 */
-	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
-	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
-	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF0_RDY);
-
-	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
-	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
-	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF1_RDY);
-
 	spin_unlock_irqrestore(&ipu->lock, flags);
 
 	return 0;
@@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
 
 	/* Other interrupts do not interfere with this channel */
 	spin_lock(&ichan->lock);
-	if (unlikely(chan_id != IDMAC_SDC_0 && chan_id != IDMAC_SDC_1 &&
-		     ((curbuf >> chan_id) & 1) == ichan->active_buffer &&
-		     !list_is_last(ichan->queue.next, &ichan->queue))) {
-		int i = 100;
-
-		/* This doesn't help. See comment in ipu_disable_channel() */
-		while (--i) {
-			curbuf = idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
-			if (((curbuf >> chan_id) & 1) != ichan->active_buffer)
-				break;
-			cpu_relax();
-		}
-
-		if (!i) {
-			spin_unlock(&ichan->lock);
-			dev_dbg(dev,
-				"IRQ on active buffer on channel %x, active "
-				"%d, ready %x, %x, current %x!\n", chan_id,
-				ichan->active_buffer, ready0, ready1, curbuf);
-			return IRQ_NONE;
-		} else
-			dev_dbg(dev,
-				"Buffer deactivated on channel %x, active "
-				"%d, ready %x, %x, current %x, rest %d!\n", chan_id,
-				ichan->active_buffer, ready0, ready1, curbuf, i);
-	}
-
 	if (unlikely((ichan->active_buffer && (ready1 >> chan_id) & 1) ||
 		     (!ichan->active_buffer && (ready0 >> chan_id) & 1)
 		     )) {
-- 
1.7.1

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

Hi Anatolij

On Mon, 31 Jan 2011, Anatolij Gustschin wrote:

I'm afraid there seems to be a problem with your patch. I have no idea 
what is causing it, but I'm just observing some wrong behaviour, that is 
not there without it. Namely, I added a debug print to the IDMAC interrupt 
handler

 	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
 	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
 
+	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n", __func__,
+	       irq, ichan->active_buffer, (curbuf >> chan_id) & 1);
 
 	if (err & (1 << chan_id)) {
 		idmac_write_ipureg(&ipu_data, 1 << chan_id, IPU_INT_STAT_4);

and without your patch I see buffer numbers correctly toggling all the 
time like

idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 1
idmac_interrupt(): IDMAC irq 177, buf 1, current 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 1
idmac_interrupt(): IDMAC irq 177, buf 1, current 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 1
...

Yes, the first interrupt is different, that's where I'm dropping / 
postponing it. With your patch only N (equal to the number of buffers 
used, I think) first interrupts toggle, then always only one buffer is 
used:

idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
...

Verified with both capture.c and mplayer. Could you, please, verify 
whether you get the same behaviour and what the problem could be?

Thanks
Guennadi

> Currently when two or more buffers are queued by the camera driver
> and so the double buffering is enabled in the idmac, we lose one
> frame comming from CSI since the reporting of arrival of the first
> frame is deferred by the DMAIC_7_EOF interrupt handler and reporting
> of the arrival of the last frame is not done at all. So when requesting
> N frames from the image sensor we actually receive N - 1 frames in
> user space.
> 
> The reason for this behaviour is that the DMAIC_7_EOF interrupt
> handler misleadingly assumes that the CUR_BUF flag is pointing to the
> buffer used by the IDMAC. Actually it is not the case since the
> CUR_BUF flag will be flipped by the FSU when the FSU is sending the
> <TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
> When sending this singal, FSU updates the DMA_CUR_BUF and the
> DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
> is cleared, indicating that the frame data is beeing written by
> the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
> set to the ready state again by the MCU, when it has handled the
> received data. DMAIC_7_CUR_BUF flag won't be flipped here by the
> IPU, so waiting for this event in the EOF interrupt handler is wrong.
> Actually there is no spurious interrupt as described in the comments,
> this is the valid DMAIC_7_EOF interrupt indicating reception of the
> frame from CSI.
> 
> The patch removes code that waits for flipping of the DMAIC_7_CUR_BUF
> flag in the DMAIC_7_EOF interrupt handler. As the comment in the
> current code denotes, this waiting doesn't help anyway. As a result
> of this removal the reporting of the first arrived frame is not
> deferred to the time of arrival of the next frame and the drivers
> software flag 'ichan->active_buffer' is in sync with DMAIC_7_CUR_BUF
> flag, so the reception of all requested frames works.
> 
> This has been verified on the hardware which is triggering the
> image sensor by the programmable state machine, allowing to
> obtain exact number of frames. On this hardware we do not tolerate
> losing frames.
> 
> This patch also removes resetting the DMA_BUFx_RDY flags of
> all channels in ipu_disable_channel() since transfers on other
> DMA channels might be triggered by other running tasks and the
> buffers should always be ready for data sending or reception.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> v2:
>     Revise the commit message to provide more and correct
>     information about the observed problem and proposed fix
> 
>  drivers/dma/ipu/ipu_idmac.c |   50 -------------------------------------------
>  1 files changed, 0 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> index cb26ee9..c1a125e 100644
> --- a/drivers/dma/ipu/ipu_idmac.c
> +++ b/drivers/dma/ipu/ipu_idmac.c
> @@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac, struct idmac_channel *ichan,
>  	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
>  	idmac_write_icreg(ipu, reg & ~chan_mask, IDMAC_CHA_EN);
>  
> -	/*
> -	 * Problem (observed with channel DMAIC_7): after enabling the channel
> -	 * and initialising buffers, there comes an interrupt with current still
> -	 * pointing at buffer 0, whereas it should use buffer 0 first and only
> -	 * generate an interrupt when it is done, then current should already
> -	 * point to buffer 1. This spurious interrupt also comes on channel
> -	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
> -	 * first interrupt, there comes the second with current correctly
> -	 * pointing to buffer 1 this time. But sometimes this second interrupt
> -	 * doesn't come and the channel hangs. Clearing BUFx_RDY when disabling
> -	 * the channel seems to prevent the channel from hanging, but it doesn't
> -	 * prevent the spurious interrupt. This might also be unsafe. Think
> -	 * about the IDMAC controller trying to switch to a buffer, when we
> -	 * clear the ready bit, and re-enable it a moment later.
> -	 */
> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
> -	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF0_RDY);
> -
> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
> -	idmac_write_ipureg(ipu, reg & ~(1UL << channel), IPU_CHA_BUF1_RDY);
> -
>  	spin_unlock_irqrestore(&ipu->lock, flags);
>  
>  	return 0;
> @@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
>  
>  	/* Other interrupts do not interfere with this channel */
>  	spin_lock(&ichan->lock);
> -	if (unlikely(chan_id != IDMAC_SDC_0 && chan_id != IDMAC_SDC_1 &&
> -		     ((curbuf >> chan_id) & 1) == ichan->active_buffer &&
> -		     !list_is_last(ichan->queue.next, &ichan->queue))) {
> -		int i = 100;
> -
> -		/* This doesn't help. See comment in ipu_disable_channel() */
> -		while (--i) {
> -			curbuf = idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
> -			if (((curbuf >> chan_id) & 1) != ichan->active_buffer)
> -				break;
> -			cpu_relax();
> -		}
> -
> -		if (!i) {
> -			spin_unlock(&ichan->lock);
> -			dev_dbg(dev,
> -				"IRQ on active buffer on channel %x, active "
> -				"%d, ready %x, %x, current %x!\n", chan_id,
> -				ichan->active_buffer, ready0, ready1, curbuf);
> -			return IRQ_NONE;
> -		} else
> -			dev_dbg(dev,
> -				"Buffer deactivated on channel %x, active "
> -				"%d, ready %x, %x, current %x, rest %d!\n", chan_id,
> -				ichan->active_buffer, ready0, ready1, curbuf, i);
> -	}
> -
>  	if (unlikely((ichan->active_buffer && (ready1 >> chan_id) & 1) ||
>  		     (!ichan->active_buffer && (ready0 >> chan_id) & 1)
>  		     )) {
> -- 
> 1.7.1
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Markus Niebel]

Hello Guennadi, hello Anatolij

I've tried that with my setup:

Hardware: i.MX35, special CCD camera over FPGA
Kernel: 2.6.34

patch v4l: soc-camera: start stream after queueing the buffers is 
applied and our camera driver handles streamon / streamoff so that no 
sync signal / clock is provided, when not streaming.

Our setup works with 4 buffers

What we see is as we would expect plus no difference with 1st buffer:

[  206.770000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 1
[  207.350000] i5ccdhb i5ccdhb.0: i5ccdhb_streamon: fps (29.412)
[  207.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  207.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  207.540000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.580000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  207.610000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.650000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  207.680000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.710000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  207.750000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  207.780000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
...
[  241.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  241.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  241.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  241.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  241.510000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  241.540000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  241.580000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
[  241.610000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
[  257.190000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 0



Am 03.02.2011 11:09, schrieb Guennadi Liakhovetski:
> Hi Anatolij
>
> On Mon, 31 Jan 2011, Anatolij Gustschin wrote:
>
> I'm afraid there seems to be a problem with your patch. I have no idea
> what is causing it, but I'm just observing some wrong behaviour, that is
> not there without it. Namely, I added a debug print to the IDMAC interrupt
> handler
>
>   	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
>   	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
>
> +	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n", __func__,
> +	       irq, ichan->active_buffer, (curbuf>>  chan_id)&  1);
>
>   	if (err&  (1<<  chan_id)) {
>   		idmac_write_ipureg(&ipu_data, 1<<  chan_id, IPU_INT_STAT_4);
>
> and without your patch I see buffer numbers correctly toggling all the
> time like
>
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> ...
>
> Yes, the first interrupt is different, that's where I'm dropping /
> postponing it. With your patch only N (equal to the number of buffers
> used, I think) first interrupts toggle, then always only one buffer is
> used:
>
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> ...
>
> Verified with both capture.c and mplayer. Could you, please, verify
> whether you get the same behaviour and what the problem could be?
>
> Thanks
> Guennadi
>
>> Currently when two or more buffers are queued by the camera driver
>> and so the double buffering is enabled in the idmac, we lose one
>> frame comming from CSI since the reporting of arrival of the first
>> frame is deferred by the DMAIC_7_EOF interrupt handler and reporting
>> of the arrival of the last frame is not done at all. So when requesting
>> N frames from the image sensor we actually receive N - 1 frames in
>> user space.
>>
>> The reason for this behaviour is that the DMAIC_7_EOF interrupt
>> handler misleadingly assumes that the CUR_BUF flag is pointing to the
>> buffer used by the IDMAC. Actually it is not the case since the
>> CUR_BUF flag will be flipped by the FSU when the FSU is sending the
>> <TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
>> When sending this singal, FSU updates the DMA_CUR_BUF and the
>> DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
>> is cleared, indicating that the frame data is beeing written by
>> the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
>> set to the ready state again by the MCU, when it has handled the
>> received data. DMAIC_7_CUR_BUF flag won't be flipped here by the
>> IPU, so waiting for this event in the EOF interrupt handler is wrong.
>> Actually there is no spurious interrupt as described in the comments,
>> this is the valid DMAIC_7_EOF interrupt indicating reception of the
>> frame from CSI.
>>
>> The patch removes code that waits for flipping of the DMAIC_7_CUR_BUF
>> flag in the DMAIC_7_EOF interrupt handler. As the comment in the
>> current code denotes, this waiting doesn't help anyway. As a result
>> of this removal the reporting of the first arrived frame is not
>> deferred to the time of arrival of the next frame and the drivers
>> software flag 'ichan->active_buffer' is in sync with DMAIC_7_CUR_BUF
>> flag, so the reception of all requested frames works.
>>
>> This has been verified on the hardware which is triggering the
>> image sensor by the programmable state machine, allowing to
>> obtain exact number of frames. On this hardware we do not tolerate
>> losing frames.
>>
>> This patch also removes resetting the DMA_BUFx_RDY flags of
>> all channels in ipu_disable_channel() since transfers on other
>> DMA channels might be triggered by other running tasks and the
>> buffers should always be ready for data sending or reception.
>>
>> Signed-off-by: Anatolij Gustschin<agust@denx.de>
>> ---
>> v2:
>>      Revise the commit message to provide more and correct
>>      information about the observed problem and proposed fix
>>
>>   drivers/dma/ipu/ipu_idmac.c |   50 -------------------------------------------
>>   1 files changed, 0 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
>> index cb26ee9..c1a125e 100644
>> --- a/drivers/dma/ipu/ipu_idmac.c
>> +++ b/drivers/dma/ipu/ipu_idmac.c
>> @@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac, struct idmac_channel *ichan,
>>   	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
>>   	idmac_write_icreg(ipu, reg&  ~chan_mask, IDMAC_CHA_EN);
>>
>> -	/*
>> -	 * Problem (observed with channel DMAIC_7): after enabling the channel
>> -	 * and initialising buffers, there comes an interrupt with current still
>> -	 * pointing at buffer 0, whereas it should use buffer 0 first and only
>> -	 * generate an interrupt when it is done, then current should already
>> -	 * point to buffer 1. This spurious interrupt also comes on channel
>> -	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
>> -	 * first interrupt, there comes the second with current correctly
>> -	 * pointing to buffer 1 this time. But sometimes this second interrupt
>> -	 * doesn't come and the channel hangs. Clearing BUFx_RDY when disabling
>> -	 * the channel seems to prevent the channel from hanging, but it doesn't
>> -	 * prevent the spurious interrupt. This might also be unsafe. Think
>> -	 * about the IDMAC controller trying to switch to a buffer, when we
>> -	 * clear the ready bit, and re-enable it a moment later.
>> -	 */
>> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
>> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
>> -	idmac_write_ipureg(ipu, reg&  ~(1UL<<  channel), IPU_CHA_BUF0_RDY);
>> -
>> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
>> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
>> -	idmac_write_ipureg(ipu, reg&  ~(1UL<<  channel), IPU_CHA_BUF1_RDY);
>> -
>>   	spin_unlock_irqrestore(&ipu->lock, flags);
>>
>>   	return 0;
>> @@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
>>
>>   	/* Other interrupts do not interfere with this channel */
>>   	spin_lock(&ichan->lock);
>> -	if (unlikely(chan_id != IDMAC_SDC_0&&  chan_id != IDMAC_SDC_1&&
>> -		     ((curbuf>>  chan_id)&  1) == ichan->active_buffer&&
>> -		     !list_is_last(ichan->queue.next,&ichan->queue))) {
>> -		int i = 100;
>> -
>> -		/* This doesn't help. See comment in ipu_disable_channel() */
>> -		while (--i) {
>> -			curbuf = idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
>> -			if (((curbuf>>  chan_id)&  1) != ichan->active_buffer)
>> -				break;
>> -			cpu_relax();
>> -		}
>> -
>> -		if (!i) {
>> -			spin_unlock(&ichan->lock);
>> -			dev_dbg(dev,
>> -				"IRQ on active buffer on channel %x, active "
>> -				"%d, ready %x, %x, current %x!\n", chan_id,
>> -				ichan->active_buffer, ready0, ready1, curbuf);
>> -			return IRQ_NONE;
>> -		} else
>> -			dev_dbg(dev,
>> -				"Buffer deactivated on channel %x, active "
>> -				"%d, ready %x, %x, current %x, rest %d!\n", chan_id,
>> -				ichan->active_buffer, ready0, ready1, curbuf, i);
>> -	}
>> -
>>   	if (unlikely((ichan->active_buffer&&  (ready1>>  chan_id)&  1) ||
>>   		     (!ichan->active_buffer&&  (ready0>>  chan_id)&  1)
>>   		     )) {
>> --
>> 1.7.1
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

Hi Markus

On Fri, 4 Feb 2011, Markus Niebel wrote:

> Hello Guennadi, hello Anatolij
> 
> I've tried that with my setup:
> 
> Hardware: i.MX35, special CCD camera over FPGA
> Kernel: 2.6.34
> 
> patch v4l: soc-camera: start stream after queueing the buffers is applied and
> our camera driver handles streamon / streamoff so that no sync signal / clock
> is provided, when not streaming.
> 
> Our setup works with 4 buffers
> 
> What we see is as we would expect plus no difference with 1st buffer:

But you haven't applied the patch, that my reply was actually referring to 
- the change to ipu_idmac.c? I think, that's the one, killing the 
double-buffering. But thanks for testing the streamon patch too!

Thanks
Guennadi

> 
> [  206.770000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 1
> [  207.350000] i5ccdhb i5ccdhb.0: i5ccdhb_streamon: fps (29.412)
> [  207.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  207.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  207.540000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.580000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  207.610000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.650000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  207.680000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.710000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  207.750000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  207.780000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> ...
> [  241.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  241.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  241.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  241.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  241.510000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  241.540000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  241.580000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> [  241.610000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> [  257.190000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 0
> 
> 
> 
> Am 03.02.2011 11:09, schrieb Guennadi Liakhovetski:
> > Hi Anatolij
> > 
> > On Mon, 31 Jan 2011, Anatolij Gustschin wrote:
> > 
> > I'm afraid there seems to be a problem with your patch. I have no idea
> > what is causing it, but I'm just observing some wrong behaviour, that is
> > not there without it. Namely, I added a debug print to the IDMAC interrupt
> > handler
> > 
> >   	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
> >   	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
> > 
> > +	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n",
> > __func__,
> > +	       irq, ichan->active_buffer, (curbuf>>  chan_id)&  1);
> > 
> >   	if (err&  (1<<  chan_id)) {
> >   		idmac_write_ipureg(&ipu_data, 1<<  chan_id, IPU_INT_STAT_4);
> > 
> > and without your patch I see buffer numbers correctly toggling all the
> > time like
> > 
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > ...
> > 
> > Yes, the first interrupt is different, that's where I'm dropping /
> > postponing it. With your patch only N (equal to the number of buffers
> > used, I think) first interrupts toggle, then always only one buffer is
> > used:
> > 
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > ...
> > 
> > Verified with both capture.c and mplayer. Could you, please, verify
> > whether you get the same behaviour and what the problem could be?
> > 
> > Thanks
> > Guennadi
> > 
> > > Currently when two or more buffers are queued by the camera driver
> > > and so the double buffering is enabled in the idmac, we lose one
> > > frame comming from CSI since the reporting of arrival of the first
> > > frame is deferred by the DMAIC_7_EOF interrupt handler and reporting
> > > of the arrival of the last frame is not done at all. So when requesting
> > > N frames from the image sensor we actually receive N - 1 frames in
> > > user space.
> > > 
> > > The reason for this behaviour is that the DMAIC_7_EOF interrupt
> > > handler misleadingly assumes that the CUR_BUF flag is pointing to the
> > > buffer used by the IDMAC. Actually it is not the case since the
> > > CUR_BUF flag will be flipped by the FSU when the FSU is sending the
> > > <TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
> > > When sending this singal, FSU updates the DMA_CUR_BUF and the
> > > DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
> > > is cleared, indicating that the frame data is beeing written by
> > > the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
> > > set to the ready state again by the MCU, when it has handled the
> > > received data. DMAIC_7_CUR_BUF flag won't be flipped here by the
> > > IPU, so waiting for this event in the EOF interrupt handler is wrong.
> > > Actually there is no spurious interrupt as described in the comments,
> > > this is the valid DMAIC_7_EOF interrupt indicating reception of the
> > > frame from CSI.
> > > 
> > > The patch removes code that waits for flipping of the DMAIC_7_CUR_BUF
> > > flag in the DMAIC_7_EOF interrupt handler. As the comment in the
> > > current code denotes, this waiting doesn't help anyway. As a result
> > > of this removal the reporting of the first arrived frame is not
> > > deferred to the time of arrival of the next frame and the drivers
> > > software flag 'ichan->active_buffer' is in sync with DMAIC_7_CUR_BUF
> > > flag, so the reception of all requested frames works.
> > > 
> > > This has been verified on the hardware which is triggering the
> > > image sensor by the programmable state machine, allowing to
> > > obtain exact number of frames. On this hardware we do not tolerate
> > > losing frames.
> > > 
> > > This patch also removes resetting the DMA_BUFx_RDY flags of
> > > all channels in ipu_disable_channel() since transfers on other
> > > DMA channels might be triggered by other running tasks and the
> > > buffers should always be ready for data sending or reception.
> > > 
> > > Signed-off-by: Anatolij Gustschin<agust@denx.de>
> > > ---
> > > v2:
> > >      Revise the commit message to provide more and correct
> > >      information about the observed problem and proposed fix
> > > 
> > >   drivers/dma/ipu/ipu_idmac.c |   50
> > > -------------------------------------------
> > >   1 files changed, 0 insertions(+), 50 deletions(-)
> > > 
> > > diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> > > index cb26ee9..c1a125e 100644
> > > --- a/drivers/dma/ipu/ipu_idmac.c
> > > +++ b/drivers/dma/ipu/ipu_idmac.c
> > > @@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac,
> > > struct idmac_channel *ichan,
> > >   	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
> > >   	idmac_write_icreg(ipu, reg&  ~chan_mask, IDMAC_CHA_EN);
> > > 
> > > -	/*
> > > -	 * Problem (observed with channel DMAIC_7): after enabling the channel
> > > -	 * and initialising buffers, there comes an interrupt with current
> > > still
> > > -	 * pointing at buffer 0, whereas it should use buffer 0 first and only
> > > -	 * generate an interrupt when it is done, then current should already
> > > -	 * point to buffer 1. This spurious interrupt also comes on channel
> > > -	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
> > > -	 * first interrupt, there comes the second with current correctly
> > > -	 * pointing to buffer 1 this time. But sometimes this second interrupt
> > > -	 * doesn't come and the channel hangs. Clearing BUFx_RDY when
> > > disabling
> > > -	 * the channel seems to prevent the channel from hanging, but it
> > > doesn't
> > > -	 * prevent the spurious interrupt. This might also be unsafe. Think
> > > -	 * about the IDMAC controller trying to switch to a buffer, when we
> > > -	 * clear the ready bit, and re-enable it a moment later.
> > > -	 */
> > > -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
> > > -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
> > > -	idmac_write_ipureg(ipu, reg&  ~(1UL<<  channel), IPU_CHA_BUF0_RDY);
> > > -
> > > -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
> > > -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
> > > -	idmac_write_ipureg(ipu, reg&  ~(1UL<<  channel), IPU_CHA_BUF1_RDY);
> > > -
> > >   	spin_unlock_irqrestore(&ipu->lock, flags);
> > > 
> > >   	return 0;
> > > @@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void
> > > *dev_id)
> > > 
> > >   	/* Other interrupts do not interfere with this channel */
> > >   	spin_lock(&ichan->lock);
> > > -	if (unlikely(chan_id != IDMAC_SDC_0&&  chan_id != IDMAC_SDC_1&&
> > > -		     ((curbuf>>  chan_id)&  1) == ichan->active_buffer&&
> > > -		     !list_is_last(ichan->queue.next,&ichan->queue))) {
> > > -		int i = 100;
> > > -
> > > -		/* This doesn't help. See comment in ipu_disable_channel() */
> > > -		while (--i) {
> > > -			curbuf = idmac_read_ipureg(&ipu_data,
> > > IPU_CHA_CUR_BUF);
> > > -			if (((curbuf>>  chan_id)&  1) != ichan->active_buffer)
> > > -				break;
> > > -			cpu_relax();
> > > -		}
> > > -
> > > -		if (!i) {
> > > -			spin_unlock(&ichan->lock);
> > > -			dev_dbg(dev,
> > > -				"IRQ on active buffer on channel %x, active "
> > > -				"%d, ready %x, %x, current %x!\n", chan_id,
> > > -				ichan->active_buffer, ready0, ready1, curbuf);
> > > -			return IRQ_NONE;
> > > -		} else
> > > -			dev_dbg(dev,
> > > -				"Buffer deactivated on channel %x, active "
> > > -				"%d, ready %x, %x, current %x, rest %d!\n",
> > > chan_id,
> > > -				ichan->active_buffer, ready0, ready1, curbuf,
> > > i);
> > > -	}
> > > -
> > >   	if (unlikely((ichan->active_buffer&&  (ready1>>  chan_id)&  1) ||
> > >   		     (!ichan->active_buffer&&  (ready0>>  chan_id)&  1)
> > >   		     )) {
> > > --
> > > 1.7.1
> > > 
> > 
> > ---
> > Guennadi Liakhovetski, Ph.D.
> > Freelance Open-Source Software Developer
> > http://www.open-technology.de/
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-media" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Markus Niebel]

Hello Guennadi,

sorry, forget to explicitly write it in my mail, I also applied the
patch in question:

dma: ipu_idmac: do not lose valid received data in the irq handler

Will try the other way round without the patch later ...

Thanks
Markus

Am 04.02.2011 10:37, schrieb Guennadi Liakhovetski:
> Hi Markus
>
> On Fri, 4 Feb 2011, Markus Niebel wrote:
>
>> Hello Guennadi, hello Anatolij
>>
>> I've tried that with my setup:
>>
>> Hardware: i.MX35, special CCD camera over FPGA
>> Kernel: 2.6.34
>>
>> patch v4l: soc-camera: start stream after queueing the buffers is applied and
>> our camera driver handles streamon / streamoff so that no sync signal / clock
>> is provided, when not streaming.
>>
>> Our setup works with 4 buffers
>>
>> What we see is as we would expect plus no difference with 1st buffer:
>
> But you haven't applied the patch, that my reply was actually referring to
> - the change to ipu_idmac.c? I think, that's the one, killing the
> double-buffering. But thanks for testing the streamon patch too!
>
> Thanks
> Guennadi
>
>>
>> [  206.770000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 1
>> [  207.350000] i5ccdhb i5ccdhb.0: i5ccdhb_streamon: fps (29.412)
>> [  207.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  207.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  207.540000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.580000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  207.610000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.650000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  207.680000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.710000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  207.750000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  207.780000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> ...
>> [  241.370000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  241.410000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  241.440000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  241.470000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  241.510000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  241.540000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  241.580000] idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>> [  241.610000] idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>> [  257.190000] i5ccdhb i5ccdhb.0: soc_i5ccdhb_s_stream - enable 0
>>
>>
>>
>> Am 03.02.2011 11:09, schrieb Guennadi Liakhovetski:
>>> Hi Anatolij
>>>
>>> On Mon, 31 Jan 2011, Anatolij Gustschin wrote:
>>>
>>> I'm afraid there seems to be a problem with your patch. I have no idea
>>> what is causing it, but I'm just observing some wrong behaviour, that is
>>> not there without it. Namely, I added a debug print to the IDMAC interrupt
>>> handler
>>>
>>>    	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
>>>    	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
>>>
>>> +	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n",
>>> __func__,
>>> +	       irq, ichan->active_buffer, (curbuf>>   chan_id)&   1);
>>>
>>>    	if (err&   (1<<   chan_id)) {
>>>    		idmac_write_ipureg(&ipu_data, 1<<   chan_id, IPU_INT_STAT_4);
>>>
>>> and without your patch I see buffer numbers correctly toggling all the
>>> time like
>>>
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
>>> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
>>> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
>>> ...
>>>
>>> Yes, the first interrupt is different, that's where I'm dropping /
>>> postponing it. With your patch only N (equal to the number of buffers
>>> used, I think) first interrupts toggle, then always only one buffer is
>>> used:
>>>
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
>>> ...
>>>
>>> Verified with both capture.c and mplayer. Could you, please, verify
>>> whether you get the same behaviour and what the problem could be?
>>>
>>> Thanks
>>> Guennadi
>>>
>>>> Currently when two or more buffers are queued by the camera driver
>>>> and so the double buffering is enabled in the idmac, we lose one
>>>> frame comming from CSI since the reporting of arrival of the first
>>>> frame is deferred by the DMAIC_7_EOF interrupt handler and reporting
>>>> of the arrival of the last frame is not done at all. So when requesting
>>>> N frames from the image sensor we actually receive N - 1 frames in
>>>> user space.
>>>>
>>>> The reason for this behaviour is that the DMAIC_7_EOF interrupt
>>>> handler misleadingly assumes that the CUR_BUF flag is pointing to the
>>>> buffer used by the IDMAC. Actually it is not the case since the
>>>> CUR_BUF flag will be flipped by the FSU when the FSU is sending the
>>>> <TASK>_NEW_FRM_RDY signal when new frame data is delivered by the CSI.
>>>> When sending this singal, FSU updates the DMA_CUR_BUF and the
>>>> DMA_BUFx_RDY flags: the DMA_CUR_BUF is flipped, the DMA_BUFx_RDY
>>>> is cleared, indicating that the frame data is beeing written by
>>>> the IDMAC to the pointed buffer. DMA_BUFx_RDY is supposed to be
>>>> set to the ready state again by the MCU, when it has handled the
>>>> received data. DMAIC_7_CUR_BUF flag won't be flipped here by the
>>>> IPU, so waiting for this event in the EOF interrupt handler is wrong.
>>>> Actually there is no spurious interrupt as described in the comments,
>>>> this is the valid DMAIC_7_EOF interrupt indicating reception of the
>>>> frame from CSI.
>>>>
>>>> The patch removes code that waits for flipping of the DMAIC_7_CUR_BUF
>>>> flag in the DMAIC_7_EOF interrupt handler. As the comment in the
>>>> current code denotes, this waiting doesn't help anyway. As a result
>>>> of this removal the reporting of the first arrived frame is not
>>>> deferred to the time of arrival of the next frame and the drivers
>>>> software flag 'ichan->active_buffer' is in sync with DMAIC_7_CUR_BUF
>>>> flag, so the reception of all requested frames works.
>>>>
>>>> This has been verified on the hardware which is triggering the
>>>> image sensor by the programmable state machine, allowing to
>>>> obtain exact number of frames. On this hardware we do not tolerate
>>>> losing frames.
>>>>
>>>> This patch also removes resetting the DMA_BUFx_RDY flags of
>>>> all channels in ipu_disable_channel() since transfers on other
>>>> DMA channels might be triggered by other running tasks and the
>>>> buffers should always be ready for data sending or reception.
>>>>
>>>> Signed-off-by: Anatolij Gustschin<agust@denx.de>
>>>> ---
>>>> v2:
>>>>       Revise the commit message to provide more and correct
>>>>       information about the observed problem and proposed fix
>>>>
>>>>    drivers/dma/ipu/ipu_idmac.c |   50
>>>> -------------------------------------------
>>>>    1 files changed, 0 insertions(+), 50 deletions(-)
>>>>
>>>> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
>>>> index cb26ee9..c1a125e 100644
>>>> --- a/drivers/dma/ipu/ipu_idmac.c
>>>> +++ b/drivers/dma/ipu/ipu_idmac.c
>>>> @@ -1145,29 +1145,6 @@ static int ipu_disable_channel(struct idmac *idmac,
>>>> struct idmac_channel *ichan,
>>>>    	reg = idmac_read_icreg(ipu, IDMAC_CHA_EN);
>>>>    	idmac_write_icreg(ipu, reg&   ~chan_mask, IDMAC_CHA_EN);
>>>>
>>>> -	/*
>>>> -	 * Problem (observed with channel DMAIC_7): after enabling the channel
>>>> -	 * and initialising buffers, there comes an interrupt with current
>>>> still
>>>> -	 * pointing at buffer 0, whereas it should use buffer 0 first and only
>>>> -	 * generate an interrupt when it is done, then current should already
>>>> -	 * point to buffer 1. This spurious interrupt also comes on channel
>>>> -	 * DMASDC_0. With DMAIC_7 normally, is we just leave the ISR after the
>>>> -	 * first interrupt, there comes the second with current correctly
>>>> -	 * pointing to buffer 1 this time. But sometimes this second interrupt
>>>> -	 * doesn't come and the channel hangs. Clearing BUFx_RDY when
>>>> disabling
>>>> -	 * the channel seems to prevent the channel from hanging, but it
>>>> doesn't
>>>> -	 * prevent the spurious interrupt. This might also be unsafe. Think
>>>> -	 * about the IDMAC controller trying to switch to a buffer, when we
>>>> -	 * clear the ready bit, and re-enable it a moment later.
>>>> -	 */
>>>> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF0_RDY);
>>>> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF0_RDY);
>>>> -	idmac_write_ipureg(ipu, reg&   ~(1UL<<   channel), IPU_CHA_BUF0_RDY);
>>>> -
>>>> -	reg = idmac_read_ipureg(ipu, IPU_CHA_BUF1_RDY);
>>>> -	idmac_write_ipureg(ipu, 0, IPU_CHA_BUF1_RDY);
>>>> -	idmac_write_ipureg(ipu, reg&   ~(1UL<<   channel), IPU_CHA_BUF1_RDY);
>>>> -
>>>>    	spin_unlock_irqrestore(&ipu->lock, flags);
>>>>
>>>>    	return 0;
>>>> @@ -1246,33 +1223,6 @@ static irqreturn_t idmac_interrupt(int irq, void
>>>> *dev_id)
>>>>
>>>>    	/* Other interrupts do not interfere with this channel */
>>>>    	spin_lock(&ichan->lock);
>>>> -	if (unlikely(chan_id != IDMAC_SDC_0&&   chan_id != IDMAC_SDC_1&&
>>>> -		     ((curbuf>>   chan_id)&   1) == ichan->active_buffer&&
>>>> -		     !list_is_last(ichan->queue.next,&ichan->queue))) {
>>>> -		int i = 100;
>>>> -
>>>> -		/* This doesn't help. See comment in ipu_disable_channel() */
>>>> -		while (--i) {
>>>> -			curbuf = idmac_read_ipureg(&ipu_data,
>>>> IPU_CHA_CUR_BUF);
>>>> -			if (((curbuf>>   chan_id)&   1) != ichan->active_buffer)
>>>> -				break;
>>>> -			cpu_relax();
>>>> -		}
>>>> -
>>>> -		if (!i) {
>>>> -			spin_unlock(&ichan->lock);
>>>> -			dev_dbg(dev,
>>>> -				"IRQ on active buffer on channel %x, active "
>>>> -				"%d, ready %x, %x, current %x!\n", chan_id,
>>>> -				ichan->active_buffer, ready0, ready1, curbuf);
>>>> -			return IRQ_NONE;
>>>> -		} else
>>>> -			dev_dbg(dev,
>>>> -				"Buffer deactivated on channel %x, active "
>>>> -				"%d, ready %x, %x, current %x, rest %d!\n",
>>>> chan_id,
>>>> -				ichan->active_buffer, ready0, ready1, curbuf,
>>>> i);
>>>> -	}
>>>> -
>>>>    	if (unlikely((ichan->active_buffer&&   (ready1>>   chan_id)&   1) ||
>>>>    		     (!ichan->active_buffer&&   (ready0>>   chan_id)&   1)
>>>>    		     )) {
>>>> --
>>>> 1.7.1
>>>>
>>>
>>> ---
>>> Guennadi Liakhovetski, Ph.D.
>>> Freelance Open-Source Software Developer
>>> http://www.open-technology.de/
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

Hi all,

On Thu, 3 Feb 2011 11:09:54 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
...
> Yes, the first interrupt is different, that's where I'm dropping / 
> postponing it. With your patch only N (equal to the number of buffers 
> used, I think) first interrupts toggle, then always only one buffer is 
> used:
> 
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> ...
> 
> Verified with both capture.c and mplayer. Could you, please, verify 
> whether you get the same behaviour and what the problem could be?

Currently I'm quite busy, but I'll look at it this week end.

Thanks,
Anatolij

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

Hi Guennadi,

On Thu, 3 Feb 2011 11:09:54 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> Hi Anatolij
> 
> On Mon, 31 Jan 2011, Anatolij Gustschin wrote:
> 
> I'm afraid there seems to be a problem with your patch. I have no idea 
> what is causing it, but I'm just observing some wrong behaviour, that is 
> not there without it. Namely, I added a debug print to the IDMAC interrupt 
> handler
> 
>  	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
>  	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
>  
> +	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n", __func__,
> +	       irq, ichan->active_buffer, (curbuf >> chan_id) & 1);
>  
>  	if (err & (1 << chan_id)) {
>  		idmac_write_ipureg(&ipu_data, 1 << chan_id, IPU_INT_STAT_4);
> 
> and without your patch I see buffer numbers correctly toggling all the 
> time like
> 
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> ...
> 
> Yes, the first interrupt is different, that's where I'm dropping / 
> postponing it. With your patch only N (equal to the number of buffers 
> used, I think) first interrupts toggle, then always only one buffer is 
> used:
> 
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> ...
> 
> Verified with both capture.c and mplayer. Could you, please, verify 
> whether you get the same behaviour and what the problem could be?

Now I did some further testing with idmac patch applied and with
added debug print in the IDMAC interrupt handler. There is no problem.
Testing with capture.c (4 buffers used as default) shows that buffer
numbers toggle correctly for all 100 captured frames:
...
mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
...
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
mx3-camera mx3-camera.0: MX3 Camera driver detached from camera 0

Also testing with my test application didn't show any problem.
When using more than 1 buffer (tested with 2, 3 and 4 queued
buffers) double buffering works as expected and frame numbers
toggle correctly. Capturing 30 frames produce:

mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
idmac_interrupt(): IDMAC irq 177, buf 0, current 0
idmac_interrupt(): IDMAC irq 177, buf 1, current 1
mx3-camera mx3-camera.0: MX3 Camera driver detached from camera 0

Anatolij

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

On Sat, 5 Feb 2011, Anatolij Gustschin wrote:

> Hi Guennadi,
> 
> On Thu, 3 Feb 2011 11:09:54 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > Hi Anatolij
> > 
> > On Mon, 31 Jan 2011, Anatolij Gustschin wrote:
> > 
> > I'm afraid there seems to be a problem with your patch. I have no idea 
> > what is causing it, but I'm just observing some wrong behaviour, that is 
> > not there without it. Namely, I added a debug print to the IDMAC interrupt 
> > handler
> > 
> >  	curbuf	= idmac_read_ipureg(&ipu_data, IPU_CHA_CUR_BUF);
> >  	err	= idmac_read_ipureg(&ipu_data, IPU_INT_STAT_4);
> >  
> > +	printk(KERN_DEBUG "%s(): IDMAC irq %d, buf %d, current %d\n", __func__,
> > +	       irq, ichan->active_buffer, (curbuf >> chan_id) & 1);
> >  
> >  	if (err & (1 << chan_id)) {
> >  		idmac_write_ipureg(&ipu_data, 1 << chan_id, IPU_INT_STAT_4);
> > 
> > and without your patch I see buffer numbers correctly toggling all the 
> > time like
> > 
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 1
> > ...
> > 
> > Yes, the first interrupt is different, that's where I'm dropping / 
> > postponing it. With your patch only N (equal to the number of buffers 
> > used, I think) first interrupts toggle, then always only one buffer is 
> > used:
> > 
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> > ...
> > 
> > Verified with both capture.c and mplayer. Could you, please, verify 
> > whether you get the same behaviour and what the problem could be?
> 
> Now I did some further testing with idmac patch applied and with
> added debug print in the IDMAC interrupt handler. There is no problem.
> Testing with capture.c (4 buffers used as default) shows that buffer
> numbers toggle correctly for all 100 captured frames:

Hm, interesting, I'll have to look at my testing in more detail then 
(once back from FOSDEM). Could you maybe try mplayer too?

Thanks
Guennadi

> ...
> mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> ...
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> mx3-camera mx3-camera.0: MX3 Camera driver detached from camera 0
> 
> Also testing with my test application didn't show any problem.
> When using more than 1 buffer (tested with 2, 3 and 4 queued
> buffers) double buffering works as expected and frame numbers
> toggle correctly. Capturing 30 frames produce:
> 
> mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> idmac_interrupt(): IDMAC irq 177, buf 0, current 0
> idmac_interrupt(): IDMAC irq 177, buf 1, current 1
> mx3-camera mx3-camera.0: MX3 Camera driver detached from camera 0
> 
> Anatolij
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

On Sat, 5 Feb 2011 17:36:37 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
...
> > > Verified with both capture.c and mplayer. Could you, please, verify 
> > > whether you get the same behaviour and what the problem could be?
> > 
> > Now I did some further testing with idmac patch applied and with
> > added debug print in the IDMAC interrupt handler. There is no problem.
> > Testing with capture.c (4 buffers used as default) shows that buffer
> > numbers toggle correctly for all 100 captured frames:
> 
> Hm, interesting, I'll have to look at my testing in more detail then 
> (once back from FOSDEM). Could you maybe try mplayer too?

I can't try mplayer since I don't have mplayer setup for this.
But looking at the mplayer source I don't see why it should
behave differently. Depending on mode mplayer queues 2 or 6
buffers. Testing with my test app with 6 queued buffers shows
no issues, here the buffer numbers toggle correctly, too.

Anatolij

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

On Sat, 5 Feb 2011, Anatolij Gustschin wrote:

> On Sat, 5 Feb 2011 17:36:37 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> ...
> > > > Verified with both capture.c and mplayer. Could you, please, verify 
> > > > whether you get the same behaviour and what the problem could be?
> > > 
> > > Now I did some further testing with idmac patch applied and with
> > > added debug print in the IDMAC interrupt handler. There is no problem.
> > > Testing with capture.c (4 buffers used as default) shows that buffer
> > > numbers toggle correctly for all 100 captured frames:
> > 
> > Hm, interesting, I'll have to look at my testing in more detail then 
> > (once back from FOSDEM). Could you maybe try mplayer too?
> 
> I can't try mplayer since I don't have mplayer setup for this.
> But looking at the mplayer source I don't see why it should
> behave differently. Depending on mode mplayer queues 2 or 6
> buffers. Testing with my test app with 6 queued buffers shows
> no issues, here the buffer numbers toggle correctly, too.

Ok, I've done a couple more tests. With larger frames, and, therefore 
lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
tests with smaller frames and higher fps either only one buffer is used, 
or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
Could you try to verify? Without your patch with any fps buffers toggle 
consistently.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

On Mon, 7 Feb 2011 12:09:15 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
...
> > I can't try mplayer since I don't have mplayer setup for this.
> > But looking at the mplayer source I don't see why it should
> > behave differently. Depending on mode mplayer queues 2 or 6
> > buffers. Testing with my test app with 6 queued buffers shows
> > no issues, here the buffer numbers toggle correctly, too.
> 
> Ok, I've done a couple more tests. With larger frames, and, therefore 
> lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
> tests with smaller frames and higher fps either only one buffer is used, 
> or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
> Could you try to verify? Without your patch with any fps buffers toggle 
> consistently.

How small are the frames in you test? What is the highest fps value in
your test?

Thanks,
Anatolij

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

On Mon, 7 Feb 2011, Anatolij Gustschin wrote:

> On Mon, 7 Feb 2011 12:09:15 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> ...
> > > I can't try mplayer since I don't have mplayer setup for this.
> > > But looking at the mplayer source I don't see why it should
> > > behave differently. Depending on mode mplayer queues 2 or 6
> > > buffers. Testing with my test app with 6 queued buffers shows
> > > no issues, here the buffer numbers toggle correctly, too.
> > 
> > Ok, I've done a couple more tests. With larger frames, and, therefore 
> > lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
> > tests with smaller frames and higher fps either only one buffer is used, 
> > or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
> > Could you try to verify? Without your patch with any fps buffers toggle 
> > consistently.
> 
> How small are the frames in you test? What is the highest fps value in
> your test?

QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
Just try different frams sizes, go down to 64x48 or something.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Detlev Zundel]

Hi Guennadi,

>> How small are the frames in you test? What is the highest fps value in
>> your test?
>
> QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
> Just try different frams sizes, go down to 64x48 or something.

Is this a "real" usage scenario?  It feels that this is not what most
users will do and it certainly is not relevant for our application.  

Is it possible that if you are interested in such a scenario that you do
the testing?  We have spent quite a lot of time to fix the driver for
real (well full frame) capturing already and I am relucatant to spend
more time for corner cases.  Maybe we should document this as "known
limitations" of the setup?  What do you think?  I'll much rather have a
driver working for real world scenarios than for marginal test cases.

Thanks
  Detlev

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

Hi Detlev

On Mon, 7 Feb 2011, Detlev Zundel wrote:

> Hi Guennadi,
> 
> >> How small are the frames in you test? What is the highest fps value in
> >> your test?
> >
> > QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
> > Just try different frams sizes, go down to 64x48 or something.
> 
> Is this a "real" usage scenario?  It feels that this is not what most
> users will do and it certainly is not relevant for our application.  

QVGA at 25 / 50 / 60 fps is _certainly_ very much a real-life scenario.

> Is it possible that if you are interested in such a scenario that you do
> the testing?  We have spent quite a lot of time to fix the driver for
> real (well full frame) capturing already and I am relucatant to spend
> more time for corner cases.  Maybe we should document this as "known
> limitations" of the setup?  What do you think?  I'll much rather have a
> driver working for real world scenarios than for marginal test cases.

I am interested in avoiding regressions. In principle, this is a DMA 
driver, which I am not maintaining. Dan asked for my ack, so, I tested it 
and found an issue, which I would prefer to have resolved before 
committing. Of course, I don't have a decisive voice in this matter, so, 
the patch can also be merged without my ack. Otherwise - of course you 
don't have to continue testing, I will try to look at the issue as the 
time permits, and Dan will have to decide, whether he is prepared to 
commit this patch in its present form, or he would prefer this issue to be 
clarified.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Detlev Zundel]

Hi Guennadi,

> Hi Detlev
>
> On Mon, 7 Feb 2011, Detlev Zundel wrote:
>
>> Hi Guennadi,
>> 
>> >> How small are the frames in you test? What is the highest fps value in
>> >> your test?
>> >
>> > QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
>> > Just try different frams sizes, go down to 64x48 or something.
>> 
>> Is this a "real" usage scenario?  It feels that this is not what most
>> users will do and it certainly is not relevant for our application.  
>
> QVGA at 25 / 50 / 60 fps is _certainly_ very much a real-life scenario.

Yes, sure.  It was the 64x48 pixel you suggested which I believe to be
of doubtful value here.

>> Is it possible that if you are interested in such a scenario that you do
>> the testing?  We have spent quite a lot of time to fix the driver for
>> real (well full frame) capturing already and I am relucatant to spend
>> more time for corner cases.  Maybe we should document this as "known
>> limitations" of the setup?  What do you think?  I'll much rather have a
>> driver working for real world scenarios than for marginal test cases.
>
> I am interested in avoiding regressions. In principle, this is a DMA 
> driver, which I am not maintaining. Dan asked for my ack, so, I tested it 
> and found an issue, which I would prefer to have resolved before 
> committing. Of course, I don't have a decisive voice in this matter, so, 
> the patch can also be merged without my ack. Otherwise - of course you 
> don't have to continue testing, I will try to look at the issue as the 
> time permits, and Dan will have to decide, whether he is prepared to 
> commit this patch in its present form, or he would prefer this issue to be 
> clarified.

I'm fully in line with not wanting any regressions.  But is it a
regression if two independent testers report that the patch _improves_
the current situation?  As was shown by Anatolijs log, the current
driver certainly has a bug with respect to the handling of individual
frames.  This buggy behaviour only never showed up because nobody used
the driver on such a granularity.

We certainly appreciate if you can look into your scenario.

Thanks
  Detlev

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Anatolij Gustschin]

On Mon, 7 Feb 2011 12:35:44 +0100 (CET)
Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:

> On Mon, 7 Feb 2011, Anatolij Gustschin wrote:
> 
> > On Mon, 7 Feb 2011 12:09:15 +0100 (CET)
> > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > ...
> > > > I can't try mplayer since I don't have mplayer setup for this.
> > > > But looking at the mplayer source I don't see why it should
> > > > behave differently. Depending on mode mplayer queues 2 or 6
> > > > buffers. Testing with my test app with 6 queued buffers shows
> > > > no issues, here the buffer numbers toggle correctly, too.
> > > 
> > > Ok, I've done a couple more tests. With larger frames, and, therefore 
> > > lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
> > > tests with smaller frames and higher fps either only one buffer is used, 
> > > or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
> > > Could you try to verify? Without your patch with any fps buffers toggle 
> > > consistently.
> > 
> > How small are the frames in you test? What is the highest fps value in
> > your test?
> 
> QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
> Just try different frams sizes, go down to 64x48 or something.

Testing of 960x243 frames at 30 fps has been done during all my previous
tests. I didn't see any issues at 30 fps.

Anatolij

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

On Mon, 7 Feb 2011, Anatolij Gustschin wrote:

> On Mon, 7 Feb 2011 12:35:44 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > On Mon, 7 Feb 2011, Anatolij Gustschin wrote:
> > 
> > > On Mon, 7 Feb 2011 12:09:15 +0100 (CET)
> > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > > ...
> > > > > I can't try mplayer since I don't have mplayer setup for this.
> > > > > But looking at the mplayer source I don't see why it should
> > > > > behave differently. Depending on mode mplayer queues 2 or 6
> > > > > buffers. Testing with my test app with 6 queued buffers shows
> > > > > no issues, here the buffer numbers toggle correctly, too.
> > > > 
> > > > Ok, I've done a couple more tests. With larger frames, and, therefore 
> > > > lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
> > > > tests with smaller frames and higher fps either only one buffer is used, 
> > > > or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
> > > > Could you try to verify? Without your patch with any fps buffers toggle 
> > > > consistently.
> > > 
> > > How small are the frames in you test? What is the highest fps value in
> > > your test?
> > 
> > QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
> > Just try different frams sizes, go down to 64x48 or something.
> 
> Testing of 960x243 frames at 30 fps has been done during all my previous
> tests. I didn't see any issues at 30 fps.

Thanks for the clarification. You certainly realise, that your frame is 3 
times as large as mine. Sorry for not mentioning this before, I do 
appreciate your effort, and have a generally positive feeling at least 
regarding your IDMAC patch, but I don't want to just trust my feeling, 
that's why I tried to study the patch a bit more carefully. I also 
understand that you don't have infinite time to dedicate to this work. 
Neither do I. So, I will try as good as I can to try to find out the 
reason for this behaviour, any help from your side is greatly appreciated. 
But if we don't clarify this before the 2.6.39 merge window, I'm not sure, 
whether it would be smart to commit the patch as is.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Guennadi Liakhovetski]

On Mon, 7 Feb 2011, Anatolij Gustschin wrote:

> On Mon, 7 Feb 2011 12:35:44 +0100 (CET)
> Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> 
> > On Mon, 7 Feb 2011, Anatolij Gustschin wrote:
> > 
> > > On Mon, 7 Feb 2011 12:09:15 +0100 (CET)
> > > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> > > ...
> > > > > I can't try mplayer since I don't have mplayer setup for this.
> > > > > But looking at the mplayer source I don't see why it should
> > > > > behave differently. Depending on mode mplayer queues 2 or 6
> > > > > buffers. Testing with my test app with 6 queued buffers shows
> > > > > no issues, here the buffer numbers toggle correctly, too.
> > > > 
> > > > Ok, I've done a couple more tests. With larger frames, and, therefore 
> > > > lower fps - yes, with your patch buffers toggle correctly. Whereas in my 
> > > > tests with smaller frames and higher fps either only one buffer is used, 
> > > > or one is used much more often, than the other, e.g., 0 0 0 1 0 0 0 1 0... 
> > > > Could you try to verify? Without your patch with any fps buffers toggle 
> > > > consistently.
> > > 
> > > How small are the frames in you test? What is the highest fps value in
> > > your test?
> > 
> > QVGA, don't know fps exactly, pretty high, between 20 and 60fps, I think. 
> > Just try different frams sizes, go down to 64x48 or something.
> 
> Testing of 960x243 frames at 30 fps has been done during all my previous
> tests. I didn't see any issues at 30 fps.

Ok, I've found the reason. Buffer number repeats, when there is an 
underrun, which is happening in my tests, when frames are arriving quickly 
enough, but the user-space is not fast enough to process them, e.g., when 
it is writing them to files over NFS or even just displaying on the LCD. 
Without your patch these underruns happen just as well, they just don't 
get recognised, because there's always one buffer delayed, so, the queue 
is never empty.

Dan, please add my

Reviewed-(and-tested-)by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH 2/2 v2] dma: ipu_idmac: do not lose valid received data in the irq handler

[Dan Williams]

On Mon, Feb 7, 2011 at 8:49 AM, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> Ok, I've found the reason. Buffer number repeats, when there is an
> underrun, which is happening in my tests, when frames are arriving quickly
> enough, but the user-space is not fast enough to process them, e.g., when
> it is writing them to files over NFS or even just displaying on the LCD.
> Without your patch these underruns happen just as well, they just don't
> get recognised, because there's always one buffer delayed, so, the queue
> is never empty.
>
> Dan, please add my
>
> Reviewed-(and-tested-)by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

Thanks, applied v2.

--
Dan

[PATCH 0/2] Fix issues with frame reception from CSI on i.MX31

[Anatolij Gustschin]

On Wed, 26 Jan 2011 09:49:47 +0100
Anatolij Gustschin <agust@denx.de> wrote:

> On some camera systems we do not tolerate the losing of
> captured frames. We observed losing of the first frame
> from CSI when double buffering is used (multiple buffers
> queued by the mx3-camera driver).
> 
> The patches provide fixes for the observed problem.
> 
> Anatolij Gustschin (2):
>   v4l: soc-camera: start stream after queueing the buffers
>   dma: ipu_idmac: do not lose valid received data in the irq handler
> 
>  drivers/dma/ipu/ipu_idmac.c      |   50 --------------------------------------
>  drivers/media/video/soc_camera.c |    4 +-
>  2 files changed, 2 insertions(+), 52 deletions(-)

Here I provide some more information on what has been done
to analyse the observed problem with one lost frame when
capturing an exact number of frames.

To be able to see whether some errors by hardware problems
or channel or image sensor parameters miss-configuration
appear or not and to catch such potential errors we added
new IRQ handlers for IPU error interrupts. The patch for these
handlers is inlined below, if someone is interested in it.
This extension requires setting CONFIG_MX3_IPU_IRQS=10 in the
kernel configuration. Also debug output has been activated in
the mx3_camera driver.

Two buffers are queued by the test application, so double buffering
will be used. To simplify matters the image sensor is triggered to
deliver only 3 frames.

Here is the resulting debug log from the drivers. It is quoted,
so I can add comments on related places:
...
> [   66.560000] mx3-camera mx3-camera.0: Set SENS_CONF to b00, rate 45314285
> [   66.560000] mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
> [   66.560000] mx3-camera mx3-camera.0: Set format 960x243
> [   66.560000] ipu-core ipu-core: timeout = 0 * 10ms
> [   66.560000] ipu-core ipu-core: init channel = 7
> [   66.560000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x0
> [   66.560000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.560000] dma dma0chan7: Found channel 0x7, irq 177
> [   66.570000] Got SOF IRQ 179 on Channel 7
> [   66.570000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   66.570000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.570000] __________________________________________
> [   66.570000] mx3-camera mx3-camera.0: Sensor set 960x243
> [   66.570000] mx3-camera mx3-camera.0: requested bus width 10 bit: 0
> [   66.570000] mx3-camera mx3-camera.0: Flags cam: 0x7215 host: 0xf2fd common: 0x7215
> [   66.570000] mx3-camera mx3-camera.0: Set SENS_CONF to 700
> [   66.570000] mx3-camera mx3-camera.0: Set format 2056x1547
> [   66.580000] ipu-core ipu-core: timeout = 0 * 10ms
> [   66.580000] ipu-core ipu-core: init channel = 7
> [   66.580000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   66.580000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.580000] dma dma0chan7: Found channel 0x7, irq 177
> [   66.590000] mx3-camera mx3-camera.0: Sensor set 2056x1547
> [   66.590000] mx3-camera mx3-camera.0: requested bus width 10 bit: 0
> [   66.590000] mx3-camera mx3-camera.0: Flags cam: 0x7215 host: 0xf2fd common: 0x7215
> [   66.590000] mx3-camera mx3-camera.0: Set SENS_CONF to 700
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010070, data = 0x00000000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010071, data = 0x00004000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010072, data = 0x00000000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010073, data = 0x0A807000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010074, data = 0x00000006
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010078, data = 0x86800000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x00010079, data = 0x00000000
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x0001007A, data = 0x3E0E403B
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x0001007B, data = 0x00000002
> [   66.870000] ipu-core ipu-core: write param mem - addr = 0x0001007C, data = 0x00000000
> [   66.870000] dma dma0chan7: Submitting sg c798a7ac
> [   66.870000] dma dma0chan7: Updated sg c798a7ac on channel 0x7 buffer 0
> [   66.870000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   66.870000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.870000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   66.870000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.870000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   66.870000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   66.870000] mx3-camera mx3-camera.0: Submitted cookie 2 DMA 0x86800000
> [   66.890000] dma dma0chan7: Submitting sg c798a4ac
> [   66.890000] dma dma0chan7: Updated sg c798a4ac on channel 0x7 buffer 1
> [   66.890000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   66.890000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x3
> [   66.890000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   66.890000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x3
> [   66.890000] mx3-camera mx3-camera.0: Submitted cookie 3 DMA 0x86c00000

So far, two buffers have been prepared. When double buffering is
activated, DMAIC_7_CUR_BUF flag will be set, DMAIC_7_BUF0_RDY and
DMAIC_7_BUF1_RDY are set by the driver, indicating that the both
buffers are ready for DMA transfers. Image sensor trigger sequence is
started now.

> [   66.940000] Got SOF IRQ 179 on Channel 7
> [   66.940000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   66.940000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   66.940000] __________________________________________
> [   66.940000] Got NF ACK IRQ 178 on Channel 7
> [   66.940000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   66.940000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   66.940000] __________________________________________
> [   67.020000] Got EOF IRQ 180 on Channel 7
> [   67.020000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.020000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.020000] __________________________________________

When CSI starts to deliver image data, CSI_NF and NF ACK interrupts
are triggered. At this time we see, that the DMAIC_7_CUR_BUF flag has
been flipped to point to the first buffer 0 and the DMAIC_7_BUSY flag is
set, indicating that IDMAC is writing to the buffer 0. When the frame
data has been received, CSI_EOF interrupt is generated. At this time
IDMAC has transmitted the data to the buffer 0 and is not busy any more:
the DMAIC_7_BUSY flag is not set. DMAIC_7_CUR_BUF flag is pointing to
the buffer 0. DMAIC_7_BUF0_RDY flag has been reset when CSI_NF interrupt
occurred, indicating that the buffer 0 is being written by the DMA.
We got EOF IRQ 180 (which is CSI_EOF), the DMA tranfer is done,
the data is sitting in the buffer 0. The buffer 1 is ready for DMA
transfers and will be used when the next image frame arrives.

> [   67.020000] dma dma0chan7: IDMAC irq 177, buf 0
> [   67.020000] dma dma0chan7: IRQ on active buffer on channel 7, active 0, ready 0, 80, current 0!

Now the DMAIC_7_EOF interrupt (end of frame on the DMA channel) is
triggered and the interrupt handler should (among other things) now
call the done callback of the mx3_camera driver.
But the handler waits here for DMAIC_7_CUR_BUF to flip, and after
the wait counter becomes zero, the handler returns (DMAIC_7_CUR_BUF won't
flip here, it will flip first when the new frame data is comming and some
buffer is found to be in the ready state). The frame arrived and is
sitting in the buffer 0, but the reporting of this reception is not
done here.

> [   67.180000] Got SOF IRQ 179 on Channel 7
> [   67.180000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   67.180000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.180000] __________________________________________
> [   67.180000] Got NF ACK IRQ 178 on Channel 7
> [   67.180000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   67.180000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.180000] __________________________________________

Here we see it. CSI_NF and NF ACK interrupts occurred as the next
frame from the camera sensor is being received. The buffer 1 was
previously ready, so FSU flipped the DMAIC_7_CUR_BUF flag to point
to the buffer 1. DMAIC_7_BUF1_RDY flag is reset, the new frame data
is being written to the buffer 1, DMAIC_7_BUSY flag indicates
channel 7 DMA busy status.

> [   67.260000] Got EOF IRQ 180 on Channel 7
> [   67.260000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.260000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.260000] __________________________________________

CSI_EOF interrupt is triggered. The next frame data has been
received and is sitting in the buffer 1, DMAIC_7_BUSY is reset,
so the DMA transfer is completed. We have received two frames
so far.

> [   67.260000] dma dma0chan7: IDMAC irq 177, buf 0
> [   67.260000] dma dma0chan7: IDMAC irq 177, dma 0x86800000, next dma 0x86c00000, current 0, curbuf 0x00000080
> [   67.260000] ipu-core ipu-core: callback cookie 2, active DMA 0x86800000
> [   67.330000] dma dma0chan7: Submitting sg c798a7ac
> [   67.330000] dma dma0chan7: Updated sg c798a7ac on channel 0x7 buffer 0
> [   67.330000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.330000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.330000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.330000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.330000] mx3-camera mx3-camera.0: Submitted cookie 4 DMA 0x86800000

DMAIC_7_EOF interrupt for the second frame occured and the channels
active_buffer software flag 'ichan->active_buffer' is pointing to
the buffer 0, so the interrupt handler reported the reception of the
first frame in buffer 0 to the user space, the buffer 0 has been dequeued
and then enqueued again and is now ready for frame data reception.
DMAIC_7_BUF0_RDY flag is now set, indicating that the buffer 0 is ready.
The buffer 0 is prepared for reception of the third frame. The second
frame is still sitting in the buffer 1. The driver's software flag
'ichan->active_buffer' is flipped to 1.

> [   67.430000] Got SOF IRQ 179 on Channel 7
> [   67.430000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   67.430000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.430000] __________________________________________
> [   67.430000] Got NF ACK IRQ 178 on Channel 7
> [   67.430000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   67.430000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.430000] __________________________________________

CSI starts to deliver image data of the third frame , CSI_NF and NF ACK
interrupts are triggered. The buffer 0 was previously ready, so it became
current buffer: DMAIC_7_BUSY flag is set, DMAIC_7_CUR_BUF flag is pointing
to the buffer 0.

> [   67.510000] Got EOF IRQ 180 on Channel 7
> [   67.510000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.510000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.510000] __________________________________________

End of frame for the third frame is signalled, DMAIC_7_BUSY flag is
cleared, so reception is completed, the data of the third frame is
in buffer 0 now. The data of the second frame is still in the buffer 1.

> [   67.510000] dma dma0chan7: IDMAC irq 177, buf 1
> [   67.510000] dma dma0chan7: IDMAC irq 177, dma 0x86c00000, next dma 0x86800000, current 1, curbuf 0x00000000
> [   67.510000] ipu-core ipu-core: callback cookie 3, active DMA 0x86c00000
> [   67.570000] dma dma0chan7: Submitting sg c798a4ac
> [   67.570000] dma dma0chan7: Updated sg c798a4ac on channel 0x7 buffer 1
> [   67.570000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.570000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.570000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   67.570000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   67.570000] mx3-camera mx3-camera.0: Submitted cookie 5 DMA 0x86c00000

DMAIC_7_EOF interrupt for the third frame occured. The driver's software
flag 'ichan->active_buffer' was pointing to buffer 1, so the interrupt
handler reports the reception of the second frame. The buffer 1 is dequeued
and then enqueued again, second frame is arrived in user space. Since we
requested only three frames from the image sensor, no more frames will be
delivered and no interrupts from the CSI or DMAIC_7_EOF will occure. The
third frame has been received and is sitting in the buffer 0, but this won't
be reported. Instead of requested three frames we got only two frames.
So we can see that in order ot get N frames we have to request N + 1 frames
from the image sensor. In camera applications with continuous image stream
such behaviour might be acceptable. But in our use case we _must_ be able
to receive exact the same number of frames as we have requested.

> [   70.580000] mx3-camera mx3-camera.0: Release active DMA 0x86800000 (state 3), queue not empty
> [   70.580000] mx3-camera mx3-camera.0: free_buffer (vb=0xc798a740) 0x4138c000 3180632
> [   70.580000] mx3-camera mx3-camera.0: Release DMA 0x86c00000 (state 5), queue not empty
> [   70.580000] mx3-camera mx3-camera.0: free_buffer (vb=0xc798a440) 0x41695000 3180632


No IPU error interrupts appeared in the log, no IPU error interrupts
have been signalled:

grep ipu_irq /proc/interrupts 
176:          2     ipu_irq  IDMAC EOF 14
177:          3     ipu_irq  IDMAC EOF 7
178:          3     ipu_irq  DMAIC NF ACK
179:          4     ipu_irq  IC SOF
180:          3     ipu_irq  IC EOF
181:          0     ipu_irq  IC NFB4EOF ERR
182:          0     ipu_irq  BAYER BUF OVF ERR
183:          0     ipu_irq  ENC BUF OVF ERR
184:          0     ipu_irq  BAYER FRM LOST ERR
185:          0     ipu_irq  ENC FRM LOST ERR

PATCH '2/2 dma: ipu_idmac: do not lose valid received data in the irq handler'
fixes this problem. Using it we are able to receive exact number of
requested frames when using double buffering. The problem doesn't
appear when only one buffer is queued.

Here is the log how things look like with the patch 2/2 applied:

> [   71.800000] mx3-camera mx3-camera.0: Set SENS_CONF to b00, rate 45314285
> [   71.800000] mx3-camera mx3-camera.0: MX3 Camera driver attached to camera 0
> [   71.800000] mx3-camera mx3-camera.0: Set format 960x243
> [   71.800000] ipu-core ipu-core: timeout = 0 * 10ms
> [   71.800000] ipu-core ipu-core: init channel = 7
> [   71.800000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   71.800000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   71.800000] dma dma0chan7: Found channel 0x7, irq 177
> [   71.810000] Got SOF IRQ 179 on Channel 7
> [   71.810000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   71.810000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   71.810000] __________________________________________
> [   71.810000] mx3-camera mx3-camera.0: Sensor set 960x243
> [   71.810000] mx3-camera mx3-camera.0: requested bus width 10 bit: 0
> [   71.810000] mx3-camera mx3-camera.0: Flags cam: 0x7215 host: 0xf2fd common: 0x7215
> [   71.810000] mx3-camera mx3-camera.0: Set SENS_CONF to 700
> [   71.810000] mx3-camera mx3-camera.0: Set format 2056x1547
> [   71.810000] ipu-core ipu-core: timeout = 0 * 10ms
> [   71.810000] ipu-core ipu-core: init channel = 7
> [   71.810000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   71.810000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   71.810000] dma dma0chan7: Found channel 0x7, irq 177
> [   71.830000] mx3-camera mx3-camera.0: Sensor set 2056x1547
> [   71.830000] mx3-camera mx3-camera.0: requested bus width 10 bit: 0
> [   71.830000] mx3-camera mx3-camera.0: Flags cam: 0x7215 host: 0xf2fd common: 0x7215
> [   71.830000] mx3-camera mx3-camera.0: Set SENS_CONF to 700
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010070, data = 0x00000000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010071, data = 0x00004000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010072, data = 0x00000000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010073, data = 0x0A807000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010074, data = 0x00000006
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010078, data = 0x86800000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x00010079, data = 0x00000000
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x0001007A, data = 0x3E0E403B
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x0001007B, data = 0x00000002
> [   72.100000] ipu-core ipu-core: write param mem - addr = 0x0001007C, data = 0x00000000
> [   72.100000] dma dma0chan7: Submitting sg c798a32c
> [   72.100000] dma dma0chan7: Updated sg c798a32c on channel 0x7 buffer 0
> [   72.100000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4000, IDMAC_CHA_BUSY 0x4000
> [   72.100000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   72.100000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x0, IDMAC_CHA_EN 0x4000, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.100000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   72.100000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.100000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x0, DB_MODE 0x0, TASKS_STAT 0x3
> [   72.100000] mx3-camera mx3-camera.0: Submitted cookie 2 DMA 0x86800000
> [   72.120000] dma dma0chan7: Submitting sg c798a0ec
> [   72.120000] dma dma0chan7: Updated sg c798a0ec on channel 0x7 buffer 1
> [   72.120000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.120000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x3
> [   72.120000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.120000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x3
> [   72.120000] mx3-camera mx3-camera.0: Submitted cookie 3 DMA 0x86c00000
> [   72.170000] Got SOF IRQ 179 on Channel 7
> [   72.170000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.170000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.170000] __________________________________________
> [   72.170000] Got NF ACK IRQ 178 on Channel 7
> [   72.170000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.170000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.170000] __________________________________________
> [   72.250000] Got EOF IRQ 180 on Channel 7
> [   72.250000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.250000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.250000] __________________________________________
> [   72.250000] dma dma0chan7: IDMAC irq 177, buf 0
> [   72.250000] dma dma0chan7: IDMAC irq 177, dma 0x86800000, next dma 0x86c00000, current 0, curbuf 0x00000000
> [   72.250000] ipu-core ipu-core: callback cookie 2, active DMA 0x86800000
> [   72.300000] dma dma0chan7: Submitting sg c798a32c
> [   72.300000] dma dma0chan7: Updated sg c798a32c on channel 0x7 buffer 0
> [   72.300000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.300000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.300000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.300000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.300000] mx3-camera mx3-camera.0: Submitted cookie 4 DMA 0x86800000
> [   72.420000] Got SOF IRQ 179 on Channel 7
> [   72.420000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.420000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.420000] __________________________________________
> [   72.420000] Got NF ACK IRQ 178 on Channel 7
> [   72.420000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.420000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.420000] __________________________________________
> [   72.500000] Got EOF IRQ 180 on Channel 7
> [   72.500000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.500000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x0, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.500000] __________________________________________
> [   72.500000] dma dma0chan7: IDMAC irq 177, buf 1
> [   72.500000] dma dma0chan7: IDMAC irq 177, dma 0x86c00000, next dma 0x86800000, current 1, curbuf 0x00000080
> [   72.500000] ipu-core ipu-core: callback cookie 3, active DMA 0x86c00000
> [   72.550000] dma dma0chan7: Submitting sg c798a0ec
> [   72.550000] dma dma0chan7: Updated sg c798a0ec on channel 0x7 buffer 1
> [   72.550000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.550000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.550000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.550000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x80, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.550000] mx3-camera mx3-camera.0: Submitted cookie 5 DMA 0x86c00000
> [   72.660000] Got SOF IRQ 179 on Channel 7
> [   72.660000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.660000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.660000] __________________________________________
> [   72.660000] Got NF ACK IRQ 178 on Channel 7
> [   72.660000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4080
> [   72.660000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.660000] __________________________________________
> [   72.740000] Got EOF IRQ 180 on Channel 7
> [   72.740000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.740000] ipu-core ipu-core: BUF0_RDY 0x0, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.740000] __________________________________________
> [   72.740000] dma dma0chan7: IDMAC irq 177, buf 0
> [   72.740000] dma dma0chan7: IDMAC irq 177, dma 0x86800000, next dma 0x86c00000, current 0, curbuf 0x00000000
> [   72.740000] ipu-core ipu-core: callback cookie 4, active DMA 0x86800000
> [   72.790000] dma dma0chan7: Submitting sg c798a32c
> [   72.790000] dma dma0chan7: Updated sg c798a32c on channel 0x7 buffer 0
> [   72.790000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.790000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.790000] ipu-core ipu-core: IDMAC_CONF 0x70, IC_CONF 0x40000001, IDMAC_CHA_EN 0x4080, IDMAC_CHA_PRI 0x4080, IDMAC_CHA_BUSY 0x4000
> [   72.790000] ipu-core ipu-core: BUF0_RDY 0x80, BUF1_RDY 0x80, CUR_BUF 0x0, DB_MODE 0x80, TASKS_STAT 0x7
> [   72.790000] mx3-camera mx3-camera.0: Submitted cookie 6 DMA 0x86800000
> [   72.790000] mx3-camera mx3-camera.0: Release DMA 0x86800000 (state 5), queue not empty
> [   72.790000] mx3-camera mx3-camera.0: free_buffer (vb=0xc798a2c0) 0x4138c000 3180632
> [   72.790000] mx3-camera mx3-camera.0: Release active DMA 0x86c00000 (state 3), queue not empty
> [   72.790000] mx3-camera mx3-camera.0: free_buffer (vb=0xc798a080) 0x41695000 3180632

We can see here that alls three frames could be received and read out
by the test application.

Here is the used debug patch for interested parties:

 drivers/dma/ipu/ipu_idmac.c      |  143 +++++++++++++++++++++++++++++++++++++-
 drivers/media/video/mx3_camera.c |    2 +-
 2 files changed, 141 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index d696915..f9fa0b3 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -8,7 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-
+#define DEBUG
 #include <linux/init.h>
 #include <linux/platform_device.h>
 #include <linux/err.h>
@@ -910,6 +910,8 @@ static dma_cookie_t idmac_tx_submit(struct dma_async_tx_descriptor *tx)
 		goto dequeue;
 	}
 
+	dump_idmac_reg(ipu);
+
 	if (ichan->status < IPU_CHANNEL_ENABLED) {
 		ret = ipu_enable_channel(idmac, ichan);
 		if (ret < 0) {
@@ -1472,22 +1474,106 @@ static void idmac_terminate_all(struct dma_chan *chan)
 static irqreturn_t ic_sof_irq(int irq, void *dev_id)
 {
 	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
 	printk(KERN_DEBUG "Got SOF IRQ %d on Channel %d\n",
 	       irq, ichan->dma_chan.chan_id);
-	disable_irq_nosync(irq);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	/*disable_irq_nosync(irq);*/
 	return IRQ_HANDLED;
 }
 
 static irqreturn_t ic_eof_irq(int irq, void *dev_id)
 {
 	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
 	printk(KERN_DEBUG "Got EOF IRQ %d on Channel %d\n",
 	       irq, ichan->dma_chan.chan_id);
-	disable_irq_nosync(irq);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	/*disable_irq_nosync(irq);*/
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ic_nf_ack_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got NF ACK IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ic_nfb4_eof_err_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got NF B4 EOF ERR IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ic_bay_ovf_err_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got BAYER BUF OVF ERR IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ic_enc_ovf_err_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got ENC BUF OVF ERR IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t ic_bay_frm_lost_err_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got BAYER FRM LOST ERR IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ic_enc_frm_lost_err_irq(int irq, void *dev_id)
+{
+	struct idmac_channel *ichan = dev_id;
+	struct idmac *idmac = to_idmac(ichan->dma_chan.device);
+	struct ipu *ipu = to_ipu(idmac);
+	printk(KERN_DEBUG "Got ENC FRM LOST ERR IRQ %d on Channel %d\n",
+	       irq, ichan->dma_chan.chan_id);
+	dump_idmac_reg(ipu);
+	printk(KERN_DEBUG "__________________________________________\n");
+	return IRQ_HANDLED;
+}
+
+
 static int ic_sof = -EINVAL, ic_eof = -EINVAL;
+static int ic_nf_ack = -EINVAL, ic_nfb4_eof_err = -EINVAL;
+static int ic_bay_ovf_err = -EINVAL, ic_enc_ovf_err = -EINVAL;
+static int ic_bay_frm_lost_err = -EINVAL, ic_enc_frm_lost_err = -EINVAL;
 #endif
 
 static int idmac_alloc_chan_resources(struct dma_chan *chan)
@@ -1526,12 +1612,33 @@ static int idmac_alloc_chan_resources(struct dma_chan *chan)
 
 #ifdef DEBUG
 	if (chan->chan_id == IDMAC_IC_7) {
+		ic_nf_ack = ipu_irq_map(39);
+		if (ic_nf_ack > 0)
+			request_irq(ic_nf_ack, ic_nf_ack_irq, 0, "DMAIC NF ACK", ichan);
 		ic_sof = ipu_irq_map(69);
 		if (ic_sof > 0)
 			request_irq(ic_sof, ic_sof_irq, 0, "IC SOF", ichan);
 		ic_eof = ipu_irq_map(70);
 		if (ic_eof > 0)
 			request_irq(ic_eof, ic_eof_irq, 0, "IC EOF", ichan);
+		ic_nfb4_eof_err = ipu_irq_map(103);
+		if (ic_nfb4_eof_err > 0)
+			request_irq(ic_nfb4_eof_err, ic_nfb4_eof_err_irq, 0, "IC NFB4EOF ERR", ichan);
+
+		ic_bay_ovf_err = ipu_irq_map(128);
+		if (ic_bay_ovf_err > 0)
+			request_irq(ic_bay_ovf_err, ic_bay_ovf_err_irq, 0, "BAYER BUF OVF ERR", ichan);
+		ic_enc_ovf_err = ipu_irq_map(129);
+		if (ic_enc_ovf_err > 0)
+			request_irq(ic_enc_ovf_err, ic_enc_ovf_err_irq, 0, "ENC BUF OVF ERR", ichan);
+
+		ic_bay_frm_lost_err = ipu_irq_map(139);
+		if (ic_bay_frm_lost_err > 0)
+			request_irq(ic_bay_frm_lost_err, ic_bay_frm_lost_err_irq, 0, "BAYER FRM LOST ERR", ichan);
+
+		ic_enc_frm_lost_err = ipu_irq_map(140);
+		if (ic_enc_frm_lost_err > 0)
+			request_irq(ic_enc_frm_lost_err, ic_enc_frm_lost_err_irq, 0, "ENC FRM LOST ERR", ichan);
 	}
 #endif
 
@@ -1562,6 +1669,11 @@ static void idmac_free_chan_resources(struct dma_chan *chan)
 	if (ichan->status > IPU_CHANNEL_FREE) {
 #ifdef DEBUG
 		if (chan->chan_id == IDMAC_IC_7) {
+			if (ic_nf_ack > 0) {
+				free_irq(ic_nf_ack, ichan);
+				ipu_irq_unmap(39);
+				ic_nf_ack = -EINVAL;
+			}
 			if (ic_sof > 0) {
 				free_irq(ic_sof, ichan);
 				ipu_irq_unmap(69);
@@ -1572,6 +1684,31 @@ static void idmac_free_chan_resources(struct dma_chan *chan)
 				ipu_irq_unmap(70);
 				ic_eof = -EINVAL;
 			}
+			if (ic_nfb4_eof_err > 0) {
+				free_irq(ic_nfb4_eof_err, ichan);
+				ipu_irq_unmap(103);
+				ic_nfb4_eof_err = -EINVAL;
+			}
+			if (ic_bay_ovf_err > 0) {
+				free_irq(ic_bay_ovf_err, ichan);
+				ipu_irq_unmap(128);
+				ic_bay_ovf_err = -EINVAL;
+			}
+			if (ic_enc_ovf_err > 0) {
+				free_irq(ic_enc_ovf_err, ichan);
+				ipu_irq_unmap(129);
+				ic_enc_ovf_err = -EINVAL;
+			}
+			if (ic_bay_frm_lost_err > 0) {
+				free_irq(ic_bay_frm_lost_err, ichan);
+				ipu_irq_unmap(139);
+				ic_bay_frm_lost_err = -EINVAL;
+			}
+			if (ic_enc_frm_lost_err > 0) {
+				free_irq(ic_enc_frm_lost_err, ichan);
+				ipu_irq_unmap(140);
+				ic_enc_frm_lost_err = -EINVAL;
+			}
 		}
 #endif
 		free_irq(ichan->eof_irq, ichan);
diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 2322798..9e566dd 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -8,7 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
-
+#define DEBUG
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/version.h>
-- 
1.7.1