Re: [PATCH 4/4] media i.MX27 camera: handle overflows properly.
Hi Guennadi,
On 25 January 2012 13:17, Guennadi Liakhovetski g.liakhovet...@gmx.de wrote:
On Fri, 20 Jan 2012, Javier Martin wrote:
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
---
drivers/media/video/mx2_camera.c | 23 +--
1 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c
b/drivers/media/video/mx2_camera.c
index e0c5dd4..cdc614f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1274,7 +1274,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
buf-state = state;
do_gettimeofday(vb-v4l2_buf.timestamp);
vb-v4l2_buf.sequence = pcdev-frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (state == MX2_STATE_ERROR)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev-frame_count++;
@@ -1309,19 +1312,11 @@ static irqreturn_t mx27_camera_emma_irq(int
irq_emma, void *data)
struct mx2_buffer *buf;
if (status (1 7)) { /* overflow */
- u32 cntl;
- /*
- * We only disable channel 1 here since this is the only
- * enabled channel
- *
- * FIXME: the correct DMA overflow handling should be resetting
- * the buffer, returning an error frame, and continuing with
- * the next one.
- */
- cntl = readl(pcdev-base_emma + PRP_CNTL);
- writel(cntl ~(PRP_CNTL_CH1EN | PRP_CNTL_CH2EN),
- pcdev-base_emma + PRP_CNTL);
- writel(cntl, pcdev-base_emma + PRP_CNTL);
+ buf = list_entry(pcdev-active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf-bufnum, MX2_STATE_ERROR);
+ status = ~(1 7);
}
if status (3 5)) == (3 5)) ||
Does it make sense continuing processing here, if an error occurred? To me
all the four if statements in this function seem mutually-exclusive and
should be handled by a
if () {
} else if () {
...
chain.
((status (3 3)) == (3 3)))
Yes, as you point out, everything is mutually exclusive.
Regards.
--
Javier Martin
Vista Silicon S.L.
CDTUC - FASE C - Oficina S-345
Avda de los Castros s/n
39005- Santander. Cantabria. Spain
+34 942 25 32 60
www.vista-silicon.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] media i.MX27 camera: handle overflows properly.
On Fri, 20 Jan 2012, Javier Martin wrote:
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
---
drivers/media/video/mx2_camera.c | 23 +--
1 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c
b/drivers/media/video/mx2_camera.c
index e0c5dd4..cdc614f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1274,7 +1274,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
buf-state = state;
do_gettimeofday(vb-v4l2_buf.timestamp);
vb-v4l2_buf.sequence = pcdev-frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (state == MX2_STATE_ERROR)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev-frame_count++;
@@ -1309,19 +1312,11 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
struct mx2_buffer *buf;
if (status (1 7)) { /* overflow */
- u32 cntl;
- /*
- * We only disable channel 1 here since this is the only
- * enabled channel
- *
- * FIXME: the correct DMA overflow handling should be resetting
- * the buffer, returning an error frame, and continuing with
- * the next one.
- */
- cntl = readl(pcdev-base_emma + PRP_CNTL);
- writel(cntl ~(PRP_CNTL_CH1EN | PRP_CNTL_CH2EN),
-pcdev-base_emma + PRP_CNTL);
- writel(cntl, pcdev-base_emma + PRP_CNTL);
+ buf = list_entry(pcdev-active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf-bufnum, MX2_STATE_ERROR);
+ status = ~(1 7);
}
if status (3 5)) == (3 5)) ||
Does it make sense continuing processing here, if an error occurred? To me
all the four if statements in this function seem mutually-exclusive and
should be handled by a
if () {
} else if () {
...
chain.
((status (3 3)) == (3 3)))
--
1.7.0.4
Thanks
Guennadi
---
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] media i.MX27 camera: handle overflows properly.
Signed-off-by: Javier Martin javier.mar...@vista-silicon.com
---
drivers/media/video/mx2_camera.c | 23 +--
1 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index e0c5dd4..cdc614f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1274,7 +1274,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
buf-state = state;
do_gettimeofday(vb-v4l2_buf.timestamp);
vb-v4l2_buf.sequence = pcdev-frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (state == MX2_STATE_ERROR)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev-frame_count++;
@@ -1309,19 +1312,11 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
struct mx2_buffer *buf;
if (status (1 7)) { /* overflow */
- u32 cntl;
- /*
-* We only disable channel 1 here since this is the only
-* enabled channel
-*
-* FIXME: the correct DMA overflow handling should be resetting
-* the buffer, returning an error frame, and continuing with
-* the next one.
-*/
- cntl = readl(pcdev-base_emma + PRP_CNTL);
- writel(cntl ~(PRP_CNTL_CH1EN | PRP_CNTL_CH2EN),
- pcdev-base_emma + PRP_CNTL);
- writel(cntl, pcdev-base_emma + PRP_CNTL);
+ buf = list_entry(pcdev-active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf-bufnum, MX2_STATE_ERROR);
+ status = ~(1 7);
}
if status (3 5)) == (3 5)) ||
((status (3 3)) == (3 3)))
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
