[PATCH 0/4] media i.MX27 camera: fix buffer handling and videobuf2 support.
The way video buffer handling is programmed for i.MX27 leads to buffers being written when they are not ready. It can be easily checked enabling DEBUG features of the driver. This series migrate the driver to videobuf2 and provide an additional discard queue to make sure all the events are handled in the right order. I've only tested the series with an i.MX27 device and so I've tried not to touch code scpecific for mx25. However, any mx25 tester would be more than welcome. [PATCH 1/4] media i.MX27 camera: migrate driver to videobuf2 [PATCH 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks. [PATCH 3/4] media i.MX27 camera: improve discard buffer handling. [PATCH 4/4] media i.MX27 camera: handle overflows properly. -- 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 1/4] media i.MX27 camera: migrate driver to videobuf2
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 277 ++
1 files changed, 128 insertions(+), 149 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 68038e7..290ac9d 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008, Sascha Hauer, Pengutronix
* Copyright (C) 2010, Baruch Siach, Orex Computed Radiography
+ * Copyright (C) 2012, Javier Martin, Vista Silicon S.L.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -30,8 +31,8 @@
#include
#include
-#include
-#include
+#include
+#include
#include
#include
@@ -256,13 +257,25 @@ struct mx2_camera_dev {
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
u32 frame_count;
+ struct vb2_alloc_ctx*alloc_ctx;
+};
+
+enum mx2_buffer_state {
+ MX2_STATE_NEEDS_INIT = 0,
+ MX2_STATE_PREPARED = 1,
+ MX2_STATE_QUEUED = 2,
+ MX2_STATE_ACTIVE = 3,
+ MX2_STATE_DONE = 4,
+ MX2_STATE_ERROR = 5,
+ MX2_STATE_IDLE = 6,
};
/* buffer for one video frame */
struct mx2_buffer {
/* common v4l buffer stuff -- must be first */
- struct videobuf_buffer vb;
-
+ struct vb2_buffer vb;
+ struct list_headqueue;
+ enum mx2_buffer_state state;
enum v4l2_mbus_pixelcodecode;
int bufnum;
@@ -407,7 +420,7 @@ static irqreturn_t mx27_camera_irq(int irq_csi, void *data)
static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
int state)
{
- struct videobuf_buffer *vb;
+ struct vb2_buffer *vb;
struct mx2_buffer *buf;
struct mx2_buffer **fb_active = fb == 1 ? &pcdev->fb1_active :
&pcdev->fb2_active;
@@ -420,25 +433,24 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
goto out;
vb = &(*fb_active)->vb;
- dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
- vb, vb->baddr, vb->bsize);
+ dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%p %lu\n", __func__,
+ vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
- vb->state = state;
- do_gettimeofday(&vb->ts);
- vb->field_count++;
-
- wake_up(&vb->done);
+ do_gettimeofday(&vb->v4l2_buf.timestamp);
+ vb->v4l2_buf.sequence++;
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
if (list_empty(&pcdev->capture)) {
buf = NULL;
writel(0, pcdev->base_csi + fb_reg);
} else {
buf = list_entry(pcdev->capture.next, struct mx2_buffer,
- vb.queue);
+ queue);
vb = &buf->vb;
- list_del(&vb->queue);
- vb->state = VIDEOBUF_ACTIVE;
- writel(videobuf_to_dma_contig(vb), pcdev->base_csi + fb_reg);
+ list_del(&buf->queue);
+ buf->state = MX2_STATE_ACTIVE;
+ writel(vb2_dma_contig_plane_dma_addr(vb, 0),
+ pcdev->base_csi + fb_reg);
}
*fb_active = buf;
@@ -453,9 +465,9 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
u32 status = readl(pcdev->base_csi + CSISR);
if (status & CSISR_DMA_TSF_FB1_INT)
- mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 1, MX2_STATE_DONE);
else if (status & CSISR_DMA_TSF_FB2_INT)
- mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 2, MX2_STATE_DONE);
/* FIXME: handle CSISR_RFF_OR_INT */
@@ -467,59 +479,47 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void
*data)
/*
* Videobuf operations
*/
-static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
- unsigned int *size)
+static int mx2_videobuf_setup(struct vb2_queue *vq,
+ const struct v4l2_format *fmt,
+ unsigned int *count, unsigned int *num_planes,
+ unsigned int sizes[], void *alloc_ctxs[])
{
- struct soc_camera_device *icd = vq->priv_data;
+ struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
+ struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
icd->current_fmt->host_f
[PATCH 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks.
Add "start_stream" and "stop_stream" callback in order to enable
and disable the eMMa-PrP properly and save CPU usage avoiding
IRQs when the device is not streaming.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 107 +++---
1 files changed, 77 insertions(+), 30 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 290ac9d..4816da6 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -560,7 +560,6 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
struct soc_camera_host *ici =
to_soc_camera_host(icd->parent);
struct mx2_camera_dev *pcdev = ici->priv;
- struct mx2_fmt_cfg *prp = pcdev->emma_prp;
struct mx2_buffer *buf = container_of(vb, struct mx2_buffer, vb);
unsigned long flags;
@@ -572,29 +571,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
buf->state = MX2_STATE_QUEUED;
list_add_tail(&buf->queue, &pcdev->capture);
- if (mx27_camera_emma(pcdev)) {
- if (prp->cfg.channel == 1) {
- writel(PRP_CNTL_CH1EN |
- PRP_CNTL_CSIEN |
- prp->cfg.in_fmt |
- prp->cfg.out_fmt |
- PRP_CNTL_CH1_LEN |
- PRP_CNTL_CH1BYP |
- PRP_CNTL_CH1_TSKIP(0) |
- PRP_CNTL_IN_TSKIP(0),
- pcdev->base_emma + PRP_CNTL);
- } else {
- writel(PRP_CNTL_CH2EN |
- PRP_CNTL_CSIEN |
- prp->cfg.in_fmt |
- prp->cfg.out_fmt |
- PRP_CNTL_CH2_LEN |
- PRP_CNTL_CH2_TSKIP(0) |
- PRP_CNTL_IN_TSKIP(0),
- pcdev->base_emma + PRP_CNTL);
- }
- goto out;
- } else { /* cpu_is_mx25() */
+ if (!mx27_camera_emma(pcdev)) { /* cpu_is_mx25() */
u32 csicr3, dma_inten = 0;
if (pcdev->fb1_active == NULL) {
@@ -629,8 +606,6 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
writel(csicr3, pcdev->base_csi + CSICR3);
}
}
-
-out:
spin_unlock_irqrestore(&pcdev->lock, flags);
}
@@ -692,11 +667,83 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static int mx2_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (mx27_camera_emma(pcdev)) {
+ if (count < 2) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (prp->cfg.channel == 1) {
+ writel(PRP_CNTL_CH1EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH1_LEN |
+ PRP_CNTL_CH1BYP |
+ PRP_CNTL_CH1_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ } else {
+ writel(PRP_CNTL_CH2EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH2_LEN |
+ PRP_CNTL_CH2_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ }
+ }
+err:
+ spin_unlock_irqrestore(&pcdev->lock, flags);
+
+ return ret;
+}
+
+static int mx2_stop_streaming(struct vb2_queue *q)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+ unsigned long flags;
+ u32 cntl;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (mx27_camera_emma(pcdev)) {
+ cntl = readl(pcdev->base_emma + PRP_CNTL);
+
[PATCH 3/4] media i.MX27 camera: improve discard buffer handling.
The way discard buffer was previously handled lead
to possible races that made a buffer that was not
yet ready to be overwritten by new video data. This
is easily detected at 25fps just adding "#define DEBUG"
to enable the "memset" check and seeing how the image
is corrupted.
A new "discard" queue and two discard buffers have
been added to make them flow trough the pipeline
of queues and thus provide suitable event ordering.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 215 +-
1 files changed, 117 insertions(+), 98 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 4816da6..e0c5dd4 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -224,6 +224,28 @@ struct mx2_fmt_cfg {
struct mx2_prp_cfg cfg;
};
+enum mx2_buffer_state {
+ MX2_STATE_NEEDS_INIT = 0,
+ MX2_STATE_PREPARED = 1,
+ MX2_STATE_QUEUED = 2,
+ MX2_STATE_ACTIVE = 3,
+ MX2_STATE_DONE = 4,
+ MX2_STATE_ERROR = 5,
+ MX2_STATE_IDLE = 6,
+};
+
+/* buffer for one video frame */
+struct mx2_buffer {
+ /* common v4l buffer stuff -- must be first */
+ struct vb2_buffer vb;
+ struct list_headqueue;
+ enum mx2_buffer_state state;
+ enum v4l2_mbus_pixelcodecode;
+
+ int bufnum;
+ booldiscard;
+};
+
struct mx2_camera_dev {
struct device *dev;
struct soc_camera_host soc_host;
@@ -240,6 +262,7 @@ struct mx2_camera_dev {
struct list_headcapture;
struct list_headactive_bufs;
+ struct list_headdiscard;
spinlock_t lock;
@@ -252,6 +275,7 @@ struct mx2_camera_dev {
u32 csicr1;
+ struct mx2_buffer buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -260,27 +284,6 @@ struct mx2_camera_dev {
struct vb2_alloc_ctx*alloc_ctx;
};
-enum mx2_buffer_state {
- MX2_STATE_NEEDS_INIT = 0,
- MX2_STATE_PREPARED = 1,
- MX2_STATE_QUEUED = 2,
- MX2_STATE_ACTIVE = 3,
- MX2_STATE_DONE = 4,
- MX2_STATE_ERROR = 5,
- MX2_STATE_IDLE = 6,
-};
-
-/* buffer for one video frame */
-struct mx2_buffer {
- /* common v4l buffer stuff -- must be first */
- struct vb2_buffer vb;
- struct list_headqueue;
- enum mx2_buffer_state state;
- enum v4l2_mbus_pixelcodecode;
-
- int bufnum;
-};
-
static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
/*
* This is a generic configuration which is valid for most
@@ -334,6 +337,29 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
return &mx27_emma_prp_table[0];
};
+static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
+unsigned long phys, int bufnum)
+{
+ u32 imgsize = pcdev->icd->user_height * pcdev->icd->user_width;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ if (prp->cfg.channel == 1) {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum);
+ } else {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_Y_PTR -
+ 0x14 * bufnum);
+ if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
+ writel(phys + imgsize,
+ pcdev->base_emma + PRP_DEST_CB_PTR -
+ 0x14 * bufnum);
+ writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
+ PRP_DEST_CR_PTR - 0x14 * bufnum);
+ }
+ }
+}
+
static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
{
unsigned long flags;
@@ -382,7 +408,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = -1;
+ pcdev->frame_count = 0;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -648,10 +674,9 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
* types.
*/
spin_lock_irqsave(&pcdev->lock, flags);
- if (buf->state == MX2_STATE_QUEUED || buf->state == MX2_STATE_ACTIVE) {
- list_del_init(&buf->queue);
- buf->state = MX2_STATE_NEEDS_INIT;
- } else if (cpu_is_mx25() && buf->state ==
[PATCH 4/4] media i.MX27 camera: handle overflows properly.
Signed-off-by: Javier Martin
---
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
Re: [PATCH v2] media i.MX27 camera: properly detect frame loss.
Hi Guennadi, thank you for your attention. I've recently sent a new patch series on top of this patch: [PATCH 0/4] media i.MX27 camera: fix buffer handling and videobuf2 support. (http://www.mail-archive.com/linux-media@vger.kernel.org/msg42255.html) Among other things, it adds videobuf2 support and adds "stream_stop" and "stream_start" callbacks which allow to enable/disable capturing of buffers at the right moment. This also makes the sequence number trick disappear and a much cleaner approach is used instead. I suggest you hold on this patch until the new series is accepted and then merge both at the same time. What do you think? -- 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 v2] media i.MX27 camera: properly detect frame loss.
Hi Guennadi, >> I suggest you hold on this patch until the new series is accepted and >> then merge both at the same time. >> >> What do you think? > > Ok, I'll be reviewing that patch series hopefully soon, and in principle > it is good, that the buffer counting will really be fixed in it, but in an > ideal world it would be better to have this your patch merged into patch > 2/4 of the series, agree? Would I be asking too much of you if I suggest > that? Feel free to explain why this wouldn't work or just reject if you're > just too tight on schedule. I'll see ifI can swallow it that way or maybe > merge myself :-) If you, Sascha, or someone else comes up with some requests or fixes to the new series I don't mind to rebase it so you can just ignore this patch, since I would have to sent a v2 version of the series anyway. 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 1/4] media i.MX27 camera: migrate driver to videobuf2
Hi Guennadi,
thank you for your review.
>> u32 frame_count;
>> + struct vb2_alloc_ctx *alloc_ctx;
>> +};
>> +
>> +enum mx2_buffer_state {
>> + MX2_STATE_NEEDS_INIT = 0,
>> + MX2_STATE_PREPARED = 1,
>> + MX2_STATE_QUEUED = 2,
>> + MX2_STATE_ACTIVE = 3,
>> + MX2_STATE_DONE = 4,
>> + MX2_STATE_ERROR = 5,
>> + MX2_STATE_IDLE = 6,
>
> Are the numerical values important? If not - please, drop. And actually,
> you don't need most of these states, I wouldn't be surprised, if you
> didn't need them at all. You might want to revise them in a future patch.
Yes, you are right, I might have been too cautious here. I will make
mx27 not to depend on these states and will try to reduce them.
However, those ones used by mx25 I can't eliminate since I don't have
the possibility to test it.
> [snip]
>
>> @@ -467,59 +479,47 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void
>> *data)
>> /*
>> * Videobuf operations
>> */
>> -static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int
>> *count,
>> - unsigned int *size)
>> +static int mx2_videobuf_setup(struct vb2_queue *vq,
>> + const struct v4l2_format *fmt,
>> + unsigned int *count, unsigned int *num_planes,
>> + unsigned int sizes[], void *alloc_ctxs[])
>> {
>> - struct soc_camera_device *icd = vq->priv_data;
>> + struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
>> + struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>> + struct mx2_camera_dev *pcdev = ici->priv;
>> int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
>> icd->current_fmt->host_fmt);
>
> You choose not to support VIDIOC_CREATE_BUFS? You have to at least return
> an error if fmt != NULL. Or consider supporting it - look at mx3_camera.c
> or sh_mobile_ceu_camera.c (btw, atmel-isi.c has to be fixed with this
> respect too). If you decide to support it, you'll also have to drop
> .buf_prepare() (see, e.g., 07f92448045a23d27dbc3ece3abcb6bafc618d43)
I'm a bit tight on schedule and would prefer just returning NULL by
now and add this in a further patch if you are so kind.
> [snip]
>
>> @@ -529,46 +529,34 @@ static int mx2_videobuf_prepare(struct videobuf_queue
>> *vq,
>> * This can be useful if you want to see if we actually fill
>> * the buffer with something
>> */
>> - memset((void *)vb->baddr, 0xaa, vb->bsize);
>> + memset((void *)vb2_plane_vaddr(vb, 0),
>> + 0xaa, vb2_get_plane_payload(vb, 0));
>> #endif
>>
>> - if (buf->code != icd->current_fmt->code ||
>> - vb->width != icd->user_width ||
>> - vb->height != icd->user_height ||
>> - vb->field != field) {
>> + if (buf->code != icd->current_fmt->code) {
>> buf->code = icd->current_fmt->code;
>> - vb->width = icd->user_width;
>> - vb->height = icd->user_height;
>> - vb->field = field;
>> - vb->state = VIDEOBUF_NEEDS_INIT;
>> + buf->state = MX2_STATE_NEEDS_INIT;
>
> This looks broken or most likely redundant to me. The check for a changed
> code was there to reallocate the buffer, doesn't seem to make much sense
> now.
Yes, this will definitely go away and will take MX2_STATE_NEEDS_INIT with it.
> [snip]
>
>> @@ -686,10 +673,10 @@ static void mx2_videobuf_release(struct videobuf_queue
>> *vq,
>> * types.
>> */
>> spin_lock_irqsave(&pcdev->lock, flags);
>> - if (vb->state == VIDEOBUF_QUEUED) {
>> - list_del(&vb->queue);
>> - vb->state = VIDEOBUF_ERROR;
>> - } else if (cpu_is_mx25() && vb->state == VIDEOBUF_ACTIVE) {
>> + if (buf->state == MX2_STATE_QUEUED || buf->state == MX2_STATE_ACTIVE) {
>> + list_del_init(&buf->queue);
>> + buf->state = MX2_STATE_NEEDS_INIT;
>> + } else if (cpu_is_mx25() && buf->state == MX2_STATE_ACTIVE) {
>
> This doesn't look right. You already have " || buf->state ==
> MX2_STATE_ACTIVE" above, so, this latter condition is never entered?
Yeah, I'm probably breaking m25 support here. This will be fixed in
the following version of my patches.
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 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks.
Hi Guennadi,
On 25 January 2012 11:26, Guennadi Liakhovetski wrote:
> As discussed before, please, merge this patch with
>
> "media i.MX27 camera: properly detect frame loss."
Yes. You can drop that patch already.
> One more cosmetic note:
>
> On Fri, 20 Jan 2012, Javier Martin wrote:
>
>> Add "start_stream" and "stop_stream" callback in order to enable
>> and disable the eMMa-PrP properly and save CPU usage avoiding
>> IRQs when the device is not streaming.
>>
>> Signed-off-by: Javier Martin
>> ---
>> drivers/media/video/mx2_camera.c | 107
>> +++---
>> 1 files changed, 77 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 290ac9d..4816da6 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -560,7 +560,6 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
>> struct soc_camera_host *ici =
>> to_soc_camera_host(icd->parent);
>> struct mx2_camera_dev *pcdev = ici->priv;
>> - struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>> struct mx2_buffer *buf = container_of(vb, struct mx2_buffer, vb);
>> unsigned long flags;
>>
>> @@ -572,29 +571,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
>> buf->state = MX2_STATE_QUEUED;
>> list_add_tail(&buf->queue, &pcdev->capture);
>>
>> - if (mx27_camera_emma(pcdev)) {
>> - if (prp->cfg.channel == 1) {
>> - writel(PRP_CNTL_CH1EN |
>> - PRP_CNTL_CSIEN |
>> - prp->cfg.in_fmt |
>> - prp->cfg.out_fmt |
>> - PRP_CNTL_CH1_LEN |
>> - PRP_CNTL_CH1BYP |
>> - PRP_CNTL_CH1_TSKIP(0) |
>> - PRP_CNTL_IN_TSKIP(0),
>> - pcdev->base_emma + PRP_CNTL);
>> - } else {
>> - writel(PRP_CNTL_CH2EN |
>> - PRP_CNTL_CSIEN |
>> - prp->cfg.in_fmt |
>> - prp->cfg.out_fmt |
>> - PRP_CNTL_CH2_LEN |
>> - PRP_CNTL_CH2_TSKIP(0) |
>> - PRP_CNTL_IN_TSKIP(0),
>> - pcdev->base_emma + PRP_CNTL);
>> - }
>> - goto out;
>> - } else { /* cpu_is_mx25() */
>> + if (!mx27_camera_emma(pcdev)) { /* cpu_is_mx25() */
>> u32 csicr3, dma_inten = 0;
>>
>> if (pcdev->fb1_active == NULL) {
>> @@ -629,8 +606,6 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
>> writel(csicr3, pcdev->base_csi + CSICR3);
>> }
>> }
>> -
>> -out:
>
> To my taste you're a bit too aggressive on blank lines;-) This also holds
> for the previous patch. Unless you absolutely have to edit your sources in
> a 24-line terminal, keeping those empty lines would be appreciated:-)
OK. I'll try to overcome my anger ^^
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 3/4] media i.MX27 camera: improve discard buffer handling.
Hi Guennadi,
On 25 January 2012 13:12, Guennadi Liakhovetski wrote:
> On Fri, 20 Jan 2012, Javier Martin wrote:
>
>> The way discard buffer was previously handled lead
>> to possible races that made a buffer that was not
>> yet ready to be overwritten by new video data. This
>> is easily detected at 25fps just adding "#define DEBUG"
>> to enable the "memset" check and seeing how the image
>> is corrupted.
>>
>> A new "discard" queue and two discard buffers have
>> been added to make them flow trough the pipeline
>> of queues and thus provide suitable event ordering.
>
> Hmm, do I understand it right, that the problem is, that while the first
> frame went to the discard buffer, the second one went already to a user
> buffer, while it wasn't ready yet?
The problem is not only related to the discard buffer but also the way
valid buffers were handled in an unsafe basis.
For instance, the "buf->bufnum = !bufnum" issue. If you receive and
IRQ from bufnum = 0 you have to update buffer 0, not buffer 1.
>And you solve this by adding one more
> discard buffer? Wouldn't it be possible to either not start capture until
> .start_streaming() is issued, which should also be the case after your
> patch 2/4, or, at least, just reuse one discard buffer multiple times
> until user buffers are available?
>
> If I understand right, you don't really introduce two discard buffers,
> there's still only one data buffer, but you add two discard data objects
> and a list to keep them on. TBH, this seems severely over-engineered to
> me. What's wrong with just keeping one DMA data buffer and using it as
> long, as needed, and checking in your ISR, whether a proper buffer is
> present, by looking for list_empty(active)?
>
> I added a couple of comments below, but my biggest question really is -
> why are these two buffer objects needed? Please, consider getting rid of
> them. So, this is not a full review, if the objects get removed, most of
> this patch will change anyway.
1. Why a discard buffer is needed?
When I first took a look at the code it only supported CH1 of the PrP
(i.e. RGB formats) and a discard buffer was used. My first reaction
was trying to get rid of that trick. Both CH1 and CH2 of the PrP have
two pointers that the engine uses to write video frames in a ping-pong
basis. However, there is a big difference between both channels: if
you want to detect frame loss in CH1 you have to continually feed the
two pointers with valid memory areas because the engine is always
writing and you can't stop it, and this is where the discard buffer
function is required, but CH2 has a mechanism to stop capturing and
keep counting loss frames, thus a discard buffer wouldn't be needed.
To sum up: the driver would be much cleaner without this discard
buffer trick but we would have to drop support for CH1 (RGB format).
Being respectful to CH1 support I decided to add CH2 by extending the
discard buffer strategy to both channels, since the code is cleaner
this way and doesn't make sense to manage both channels differently.
2. Why two discard buffer objects are needed?
After enabling the DEBUG functionality that writes the buffers with
0xaa before they are filled with video data, I discovered some of them
were being corrupted. When I tried to find the reason I found that we
really have a pipeline here:
--- ---
capture (n) | > active_bufs (2)|
---
where
"capture" has buffers that are queued and ready to be written into
"active_bufs" represents those buffers that are assigned to a pointer
in the PrP and has a maximum of 2 since there are two pointers that
are written in a ping-pong fashion
However, with the previous approach, the discard buffer is kept
outside this pipeline as if it was a global variable which is usually
a dangerous practice and definitely it's wrong since the buffers are
corrupted.
On the other hand, if we add 2 discard buffer objects we will be able
to pass them through the pipeline as if they were normal buffers. We
chose 2 because this is the number of pointers of the PrP and thus,
the maximum of elements that can be in "active_bufs" queue (i.e. we
have to cover the case where both pointers are assigned discard
buffers). This has several advantages:
- They can be treated in most cases as normal buffers which saves
code ("mx27_update_emma_buf" function).
- The events are properly ordered: every time an IRQ comes you know
the first element in active_bufs queue is the buffer you want, if it
is not the case something has gone terribly wrong.
- It's much easier to demonstrate mathematically that this will work
(I wasn't abl
Re: [PATCH 4/4] media i.MX27 camera: handle overflows properly.
Hi Guennadi,
On 25 January 2012 13:17, Guennadi Liakhovetski wrote:
> On Fri, 20 Jan 2012, Javier Martin wrote:
>
>>
>> Signed-off-by: Javier Martin
>> ---
>> 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
[PATCH v2 1/4] media i.MX27 camera: migrate driver to videobuf2
Signed-off-by: Javier Martin
---
Changes since v1:
- mx27 code doesn't use states.
- number of states reduced to the ones used by mx25.
- Fix incorrect if which broke mx25 support.
- Minor fixes.
---
drivers/media/video/mx2_camera.c | 298 --
1 files changed, 127 insertions(+), 171 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index ca76dd2..898f98f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008, Sascha Hauer, Pengutronix
* Copyright (C) 2010, Baruch Siach, Orex Computed Radiography
+ * Copyright (C) 2012, Javier Martin, Vista Silicon S.L.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -30,8 +31,8 @@
#include
#include
-#include
-#include
+#include
+#include
#include
#include
@@ -223,6 +224,22 @@ struct mx2_fmt_cfg {
struct mx2_prp_cfg cfg;
};
+enum mx2_buffer_state {
+ MX2_STATE_QUEUED,
+ MX2_STATE_ACTIVE,
+ MX2_STATE_DONE,
+};
+
+/* buffer for one video frame */
+struct mx2_buffer {
+ /* common v4l buffer stuff -- must be first */
+ struct vb2_buffer vb;
+ struct list_headqueue;
+ enum mx2_buffer_state state;
+
+ int bufnum;
+};
+
struct mx2_camera_dev {
struct device *dev;
struct soc_camera_host soc_host;
@@ -256,16 +273,7 @@ struct mx2_camera_dev {
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
u32 frame_count;
-};
-
-/* buffer for one video frame */
-struct mx2_buffer {
- /* common v4l buffer stuff -- must be first */
- struct videobuf_buffer vb;
-
- enum v4l2_mbus_pixelcodecode;
-
- int bufnum;
+ struct vb2_alloc_ctx*alloc_ctx;
};
static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
@@ -407,7 +415,7 @@ static irqreturn_t mx27_camera_irq(int irq_csi, void *data)
static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
int state)
{
- struct videobuf_buffer *vb;
+ struct vb2_buffer *vb;
struct mx2_buffer *buf;
struct mx2_buffer **fb_active = fb == 1 ? &pcdev->fb1_active :
&pcdev->fb2_active;
@@ -420,25 +428,24 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
goto out;
vb = &(*fb_active)->vb;
- dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
- vb, vb->baddr, vb->bsize);
+ dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%p %lu\n", __func__,
+ vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
- vb->state = state;
- do_gettimeofday(&vb->ts);
- vb->field_count++;
-
- wake_up(&vb->done);
+ do_gettimeofday(&vb->v4l2_buf.timestamp);
+ vb->v4l2_buf.sequence++;
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
if (list_empty(&pcdev->capture)) {
buf = NULL;
writel(0, pcdev->base_csi + fb_reg);
} else {
buf = list_entry(pcdev->capture.next, struct mx2_buffer,
- vb.queue);
+ queue);
vb = &buf->vb;
- list_del(&vb->queue);
- vb->state = VIDEOBUF_ACTIVE;
- writel(videobuf_to_dma_contig(vb), pcdev->base_csi + fb_reg);
+ list_del(&buf->queue);
+ buf->state = MX2_STATE_ACTIVE;
+ writel(vb2_dma_contig_plane_dma_addr(vb, 0),
+ pcdev->base_csi + fb_reg);
}
*fb_active = buf;
@@ -453,9 +460,9 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
u32 status = readl(pcdev->base_csi + CSISR);
if (status & CSISR_DMA_TSF_FB1_INT)
- mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 1, MX2_STATE_DONE);
else if (status & CSISR_DMA_TSF_FB2_INT)
- mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 2, MX2_STATE_DONE);
/* FIXME: handle CSISR_RFF_OR_INT */
@@ -467,59 +474,50 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void
*data)
/*
* Videobuf operations
*/
-static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
- unsigned int *size)
+static int mx2_videobuf_setup(struct vb2_queue *vq,
+ const struct v4l2_format *fmt,
+ unsigned int *count, unsigned int *num_planes,
+
[PATCH v2 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks.
Add "start_stream" and "stop_stream" callback in order to enable
and disable the eMMa-PrP properly and save CPU usage avoiding
IRQs when the device is not streaming. This also makes the driver
return 0 as the sequence number of the first frame.
Signed-off-by: Javier Martin
---
Merge "media i.MX27 camera: properly detect frame loss"
---
drivers/media/video/mx2_camera.c | 104 +-
1 files changed, 79 insertions(+), 25 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 898f98f..045c018 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -377,7 +377,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = 0;
+ pcdev->frame_count = -1;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -647,11 +647,83 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static int mx2_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (mx27_camera_emma(pcdev)) {
+ if (count < 2) {
+ ret = -EINVAL;
+ goto err;
+ }
+
+ if (prp->cfg.channel == 1) {
+ writel(PRP_CNTL_CH1EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH1_LEN |
+ PRP_CNTL_CH1BYP |
+ PRP_CNTL_CH1_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ } else {
+ writel(PRP_CNTL_CH2EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH2_LEN |
+ PRP_CNTL_CH2_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ }
+ }
+err:
+ spin_unlock_irqrestore(&pcdev->lock, flags);
+
+ return ret;
+}
+
+static int mx2_stop_streaming(struct vb2_queue *q)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+ unsigned long flags;
+ u32 cntl;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (mx27_camera_emma(pcdev)) {
+ cntl = readl(pcdev->base_emma + PRP_CNTL);
+ if (prp->cfg.channel == 1) {
+ writel(cntl & ~PRP_CNTL_CH1EN,
+ pcdev->base_emma + PRP_CNTL);
+ } else {
+ writel(cntl & ~PRP_CNTL_CH2EN,
+ pcdev->base_emma + PRP_CNTL);
+ }
+ }
+ spin_unlock_irqrestore(&pcdev->lock, flags);
+
+ return 0;
+}
+
static struct vb2_ops mx2_videobuf_ops = {
- .queue_setup= mx2_videobuf_setup,
- .buf_prepare= mx2_videobuf_prepare,
- .buf_queue = mx2_videobuf_queue,
- .buf_cleanup= mx2_videobuf_release,
+ .queue_setup = mx2_videobuf_setup,
+ .buf_prepare = mx2_videobuf_prepare,
+ .buf_queue = mx2_videobuf_queue,
+ .buf_cleanup = mx2_videobuf_release,
+ .start_streaming = mx2_start_streaming,
+ .stop_streaming = mx2_stop_streaming,
};
static int mx2_camera_init_videobuf(struct vb2_queue *q,
@@ -709,16 +781,6 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
writel(pcdev->discard_buffer_dma,
pcdev->base_emma + PRP_DEST_RGB2_PTR);
- writel(PRP_CNTL_CH1EN |
- PRP_CNTL_CSIEN |
- prp->cfg.in_fmt |
- prp->cfg.out_fmt |
- PRP_CNTL_CH1_LEN |
- PRP_CNTL_CH1BYP |
[PATCH v2 3/4] media i.MX27 camera: improve discard buffer handling.
The way discard buffer was previously handled lead
to possible races that made a buffer that was not
yet ready to be overwritten by new video data. This
is easily detected at 25fps just adding "#define DEBUG"
to enable the "memset" check and seeing how the image
is corrupted.
A new "discard" queue and two discard buffers have
been added to make them flow trough the pipeline
of queues and thus provide suitable event ordering.
Signed-off-by: Javier Martin
---
Changes since v1:
- Don't allocate discard buffer on every set_fmt.
---
drivers/media/video/mx2_camera.c | 261 +-
1 files changed, 144 insertions(+), 117 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 045c018..71054ab 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -237,7 +237,8 @@ struct mx2_buffer {
struct list_headqueue;
enum mx2_buffer_state state;
- int bufnum;
+ int bufnum;
+ booldiscard;
};
struct mx2_camera_dev {
@@ -256,6 +257,7 @@ struct mx2_camera_dev {
struct list_headcapture;
struct list_headactive_bufs;
+ struct list_headdiscard;
spinlock_t lock;
@@ -268,6 +270,7 @@ struct mx2_camera_dev {
u32 csicr1;
+ struct mx2_buffer buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -329,6 +332,30 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
return &mx27_emma_prp_table[0];
};
+static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
+unsigned long phys, int bufnum)
+{
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ if (prp->cfg.channel == 1) {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum);
+ } else {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_Y_PTR - 0x14 * bufnum);
+ if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
+ u32 imgsize = pcdev->icd->user_height *
+ pcdev->icd->user_width;
+
+ writel(phys + imgsize,
+ pcdev->base_emma + PRP_DEST_CB_PTR -
+ 0x14 * bufnum);
+ writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
+ PRP_DEST_CR_PTR - 0x14 * bufnum);
+ }
+ }
+}
+
static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
{
unsigned long flags;
@@ -377,7 +404,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = -1;
+ pcdev->frame_count = 0;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -631,7 +658,7 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_lock_irqsave(&pcdev->lock, flags);
if (mx27_camera_emma(pcdev)) {
- list_del_init(&buf->queue);
+ INIT_LIST_HEAD(&buf->queue);
} else if (cpu_is_mx25() && buf->state == MX2_STATE_ACTIVE) {
if (pcdev->fb1_active == buf) {
pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
@@ -647,6 +674,34 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
+ int bytesperline)
+{
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->base_emma + PRP_SRC_FRAME_SIZE);
+ writel(prp->cfg.src_pixel,
+ pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
+ if (prp->cfg.channel == 1) {
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->base_emma + PRP_CH1_OUT_IMAGE_SIZE);
+ writel(bytesperline,
+ pcdev->base_emma + PRP_DEST_CH1_LINE_STRIDE);
+ writel(prp->cfg.ch1_pixel,
+ pcdev->base_emma + PRP_CH1_PIXEL_FORMAT_CNTL);
+ } else { /* channel 2 */
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->bas
[PATCH v2 4/4] media i.MX27 camera: handle overflows properly.
Signed-off-by: Javier Martin
---
Changes since v1:
- Make ifs in irq callback mutually exclusive.
- Add new argument to mx27_camera_frame_done_emma() to handle errors.
---
drivers/media/video/mx2_camera.c | 38 --
1 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 71054ab..1759673 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1213,7 +1213,7 @@ static struct soc_camera_host_ops mx2_soc_camera_host_ops
= {
};
static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
- int bufnum)
+ int bufnum, bool err)
{
struct mx2_buffer *buf;
struct vb2_buffer *vb;
@@ -1262,7 +1262,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
list_del_init(&buf->queue);
do_gettimeofday(&vb->v4l2_buf.timestamp);
vb->v4l2_buf.sequence = pcdev->frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (err)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev->frame_count++;
@@ -1297,21 +1300,12 @@ 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);
- }
- if status & (3 << 5)) == (3 << 5)) ||
+ buf = list_entry(pcdev->active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf->bufnum, 1);
+ status &= ~(1 << 7);
+ } else if status & (3 << 5)) == (3 << 5)) ||
((status & (3 << 3)) == (3 << 3)))
&& !list_empty(&pcdev->active_bufs)) {
/*
@@ -1320,13 +1314,13 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
*/
buf = list_entry(pcdev->active_bufs.next,
struct mx2_buffer, queue);
- mx27_camera_frame_done_emma(pcdev, buf->bufnum);
+ mx27_camera_frame_done_emma(pcdev, buf->bufnum, 0);
status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
+ } else if ((status & (1 << 6)) || (status & (1 << 4))) {
+ mx27_camera_frame_done_emma(pcdev, 0, 0);
+ } else if ((status & (1 << 5)) || (status & (1 << 3))) {
+ mx27_camera_frame_done_emma(pcdev, 1, 0);
}
- if ((status & (1 << 6)) || (status & (1 << 4)))
- mx27_camera_frame_done_emma(pcdev, 0);
- if ((status & (1 << 5)) || (status & (1 << 3)))
- mx27_camera_frame_done_emma(pcdev, 1);
writel(status, pcdev->base_emma + PRP_INTRSTATUS);
--
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
Re: [PATCH v2 1/4] media i.MX27 camera: migrate driver to videobuf2
Hi Guennadi, thank you for your time. On 27 January 2012 16:25, Guennadi Liakhovetski wrote: > A general question for mx2-camera: does it now after removal of legacy > i.MX27 support only support i.MX25 (state: unknown) and i.MX27 in eMMA > mode? I understand so. I'll send a v3 of this patch fixing what you've pointed out. 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 v2 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks.
Hi,
On 27 January 2012 16:53, Guennadi Liakhovetski wrote:
> On Thu, 26 Jan 2012, Javier Martin wrote:
>
>> Add "start_stream" and "stop_stream" callback in order to enable
>> and disable the eMMa-PrP properly and save CPU usage avoiding
>> IRQs when the device is not streaming. This also makes the driver
>> return 0 as the sequence number of the first frame.
>>
>> Signed-off-by: Javier Martin
>> ---
>> Merge "media i.MX27 camera: properly detect frame loss"
>>
>> ---
>> drivers/media/video/mx2_camera.c | 104
>> +-
>> 1 files changed, 79 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 898f98f..045c018 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -377,7 +377,7 @@ static int mx2_camera_add_device(struct
>> soc_camera_device *icd)
>> writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
>>
>> pcdev->icd = icd;
>> - pcdev->frame_count = 0;
>> + pcdev->frame_count = -1;
>>
>> dev_info(icd->parent, "Camera driver attached to camera %d\n",
>> icd->devnum);
>> @@ -647,11 +647,83 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
>> spin_unlock_irqrestore(&pcdev->lock, flags);
>> }
>>
>> +static int mx2_start_streaming(struct vb2_queue *q, unsigned int count)
>> +{
>> + struct soc_camera_device *icd = soc_camera_from_vb2q(q);
>> + struct soc_camera_host *ici =
>> + to_soc_camera_host(icd->parent);
>> + struct mx2_camera_dev *pcdev = ici->priv;
>> + struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>> + unsigned long flags;
>> + int ret = 0;
>> +
>> + spin_lock_irqsave(&pcdev->lock, flags);
>> + if (mx27_camera_emma(pcdev)) {
>> + if (count < 2) {
>> + ret = -EINVAL;
>> + goto err;
>> + }
>
> How about:
>
> if (mx27_camera_emma(pcdev)) {
> unsigned long flags;
> if (count < 2)
> return -EINVAL;
>
> spin_lock_irqsave(&pcdev->lock, flags);
> ...
> spin_unlock_irqrestore(&pcdev->lock, flags);
> }
>
> return 0;
OK, this is definitely cleaner. I'll do it for v3.
> Another point: in v1 of this patch you also removed "goto out" from
> mx2_videobuf_queue(). I understand this is probably unrelated to this
> patch now. Anyway, if you don't find a patch out of your 4 now, where it
> logically would fit, you could either make an additional patch, or I could
> do it myself, if I don't forget:-)
Don't worry,
I'll send a new series including that patch after this one gets merged.
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 3/4] media i.MX27 camera: improve discard buffer handling.
Hi Guennadi, On 27 January 2012 19:02, Guennadi Liakhovetski wrote: > (removing bar...@tkos.co.il - it bounces) > > On Thu, 26 Jan 2012, javier Martin wrote: > >> Hi Guennadi, >> >> On 25 January 2012 13:12, Guennadi Liakhovetski >> wrote: >> > On Fri, 20 Jan 2012, Javier Martin wrote: >> > >> >> The way discard buffer was previously handled lead >> >> to possible races that made a buffer that was not >> >> yet ready to be overwritten by new video data. This >> >> is easily detected at 25fps just adding "#define DEBUG" >> >> to enable the "memset" check and seeing how the image >> >> is corrupted. >> >> >> >> A new "discard" queue and two discard buffers have >> >> been added to make them flow trough the pipeline >> >> of queues and thus provide suitable event ordering. >> > >> > Hmm, do I understand it right, that the problem is, that while the first >> > frame went to the discard buffer, the second one went already to a user >> > buffer, while it wasn't ready yet? >> >> The problem is not only related to the discard buffer but also the way >> valid buffers were handled in an unsafe basis. >> For instance, the "buf->bufnum = !bufnum" issue. If you receive and >> IRQ from bufnum = 0 you have to update buffer 0, not buffer 1. >> >> >And you solve this by adding one more >> > discard buffer? Wouldn't it be possible to either not start capture until >> > .start_streaming() is issued, which should also be the case after your >> > patch 2/4, or, at least, just reuse one discard buffer multiple times >> > until user buffers are available? >> > >> > If I understand right, you don't really introduce two discard buffers, >> > there's still only one data buffer, but you add two discard data objects >> > and a list to keep them on. TBH, this seems severely over-engineered to >> > me. What's wrong with just keeping one DMA data buffer and using it as >> > long, as needed, and checking in your ISR, whether a proper buffer is >> > present, by looking for list_empty(active)? >> > >> > I added a couple of comments below, but my biggest question really is - >> > why are these two buffer objects needed? Please, consider getting rid of >> > them. So, this is not a full review, if the objects get removed, most of >> > this patch will change anyway. >> >> 1. Why a discard buffer is needed? >> >> When I first took a look at the code it only supported CH1 of the PrP >> (i.e. RGB formats) and a discard buffer was used. My first reaction >> was trying to get rid of that trick. Both CH1 and CH2 of the PrP have >> two pointers that the engine uses to write video frames in a ping-pong >> basis. However, there is a big difference between both channels: if >> you want to detect frame loss in CH1 you have to continually feed the >> two pointers with valid memory areas because the engine is always >> writing and you can't stop it, and this is where the discard buffer >> function is required, but CH2 has a mechanism to stop capturing and >> keep counting loss frames, thus a discard buffer wouldn't be needed. >> >> To sum up: the driver would be much cleaner without this discard >> buffer trick but we would have to drop support for CH1 (RGB format). >> Being respectful to CH1 support I decided to add CH2 by extending the >> discard buffer strategy to both channels, since the code is cleaner >> this way and doesn't make sense to manage both channels differently. >> >> 2. Why two discard buffer objects are needed? >> >> After enabling the DEBUG functionality that writes the buffers with >> 0xaa before they are filled with video data, I discovered some of them >> were being corrupted. When I tried to find the reason I found that we >> really have a pipeline here: >> >> --- --- >> capture (n) | > active_bufs (2)| >> --- >> >> where >> "capture" has buffers that are queued and ready to be written into >> "active_bufs" represents those buffers that are assigned to a pointer >> in the PrP and has a maximum of 2 since there are two pointers that >> are written in a ping-pong fashion > > Ok, I understand what the discard memory is used for and why you need to > write it twice to the hardware - to those t
Re: [PATCH v2 3/4] media i.MX27 camera: improve discard buffer handling.
On 27 January 2012 19:13, Guennadi Liakhovetski wrote:
> On Thu, 26 Jan 2012, Javier Martin wrote:
>
>> The way discard buffer was previously handled lead
>> to possible races that made a buffer that was not
>> yet ready to be overwritten by new video data. This
>> is easily detected at 25fps just adding "#define DEBUG"
>> to enable the "memset" check and seeing how the image
>> is corrupted.
>>
>> A new "discard" queue and two discard buffers have
>> been added to make them flow trough the pipeline
>> of queues and thus provide suitable event ordering.
>>
>> Signed-off-by: Javier Martin
>> ---
>> Changes since v1:
>> - Don't allocate discard buffer on every set_fmt.
>>
>> ---
>> drivers/media/video/mx2_camera.c | 261
>> +-
>> 1 files changed, 144 insertions(+), 117 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 045c018..71054ab 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -237,7 +237,8 @@ struct mx2_buffer {
>> struct list_head queue;
>> enum mx2_buffer_state state;
>>
>> - int bufnum;
>> + int bufnum;
>> + bool discard;
>> };
>>
>> struct mx2_camera_dev {
>> @@ -256,6 +257,7 @@ struct mx2_camera_dev {
>>
>> struct list_head capture;
>> struct list_head active_bufs;
>> + struct list_head discard;
>>
>> spinlock_t lock;
>>
>> @@ -268,6 +270,7 @@ struct mx2_camera_dev {
>>
>> u32 csicr1;
>>
>> + struct mx2_buffer buf_discard[2];
>> void *discard_buffer;
>> dma_addr_t discard_buffer_dma;
>> size_t discard_size;
>> @@ -329,6 +332,30 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
>> return &mx27_emma_prp_table[0];
>> };
>>
>> +static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
>> + unsigned long phys, int bufnum)
>> +{
>> + struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>> +
>> + if (prp->cfg.channel == 1) {
>> + writel(phys, pcdev->base_emma +
>> + PRP_DEST_RGB1_PTR + 4 * bufnum);
>> + } else {
>> + writel(phys, pcdev->base_emma +
>> + PRP_DEST_Y_PTR - 0x14 * bufnum);
>> + if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
>> + u32 imgsize = pcdev->icd->user_height *
>> + pcdev->icd->user_width;
>> +
>> + writel(phys + imgsize,
>> + pcdev->base_emma + PRP_DEST_CB_PTR -
>> + 0x14 * bufnum);
>> + writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
>> + PRP_DEST_CR_PTR - 0x14 * bufnum);
>
> Please fix indentation.
OK, will do in v3.
>> + }
>> + }
>> +}
>> +
>> static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
>> {
>> unsigned long flags;
>> @@ -377,7 +404,7 @@ static int mx2_camera_add_device(struct
>> soc_camera_device *icd)
>> writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
>>
>> pcdev->icd = icd;
>> - pcdev->frame_count = -1;
>> + pcdev->frame_count = 0;
>>
>> dev_info(icd->parent, "Camera driver attached to camera %d\n",
>> icd->devnum);
>> @@ -631,7 +658,7 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
>>
>> spin_lock_irqsave(&pcdev->lock, flags);
>> if (mx27_camera_emma(pcdev)) {
>> - list_del_init(&buf->queue);
>> + INIT_LIST_HEAD(&buf->queue);
>
> The buffer had to be deleted from the queue to clean up the queue. The
> "_init" was actually not needed. Now, that you do
> INIT_LIST_HEAD(&pcdev->capture); and INIT_LIST_HEAD(&pcdev->active_bufs);
> in .stop_streaming(), you don't need to do anything about your individual
> buffers. You never test list_empty(&buf->queue), so, it doesn't matter
> what they contain.
Fine, v3 will solve this.
>> } else if (cpu_i
Re: [PATCH v2 4/4] media i.MX27 camera: handle overflows properly.
On 27 January 2012 19:16, Guennadi Liakhovetski wrote:
> (removed bar...@tkos.co.il - it bounces)
>
> On Thu, 26 Jan 2012, Javier Martin wrote:
>
>>
>> Signed-off-by: Javier Martin
>> ---
>> Changes since v1:
>> - Make ifs in irq callback mutually exclusive.
>> - Add new argument to mx27_camera_frame_done_emma() to handle errors.
>>
>> ---
>> drivers/media/video/mx2_camera.c | 38
>> --
>> 1 files changed, 16 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 71054ab..1759673 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -1213,7 +1213,7 @@ static struct soc_camera_host_ops
>> mx2_soc_camera_host_ops = {
>> };
>>
>> static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
>> - int bufnum)
>> + int bufnum, bool err)
>> {
>> struct mx2_buffer *buf;
>> struct vb2_buffer *vb;
>> @@ -1262,7 +1262,10 @@ static void mx27_camera_frame_done_emma(struct
>> mx2_camera_dev *pcdev,
>> list_del_init(&buf->queue);
>> do_gettimeofday(&vb->v4l2_buf.timestamp);
>> vb->v4l2_buf.sequence = pcdev->frame_count;
>> - vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
>> + if (err)
>> + vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
>> + else
>> + vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
>> }
>>
>> pcdev->frame_count++;
>> @@ -1297,21 +1300,12 @@ 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);
>> - }
>> - if status & (3 << 5)) == (3 << 5)) ||
>> + buf = list_entry(pcdev->active_bufs.next,
>> + struct mx2_buffer, queue);
>> + mx27_camera_frame_done_emma(pcdev,
>> + buf->bufnum, 1);
>
> use "true" for bool variables.
>
>> + status &= ~(1 << 7);
>> + } else if status & (3 << 5)) == (3 << 5)) ||
>> ((status & (3 << 3)) == (3 << 3)))
>> && !list_empty(&pcdev->active_bufs)) {
>> /*
>> @@ -1320,13 +1314,13 @@ static irqreturn_t mx27_camera_emma_irq(int
>> irq_emma, void *data)
>> */
>> buf = list_entry(pcdev->active_bufs.next,
>> struct mx2_buffer, queue);
>> - mx27_camera_frame_done_emma(pcdev, buf->bufnum);
>> + mx27_camera_frame_done_emma(pcdev, buf->bufnum, 0);
>
> "false"
>
>> status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
>> + } else if ((status & (1 << 6)) || (status & (1 << 4))) {
>> + mx27_camera_frame_done_emma(pcdev, 0, 0);
>
> "false"
>
>> + } else if ((status & (1 << 5)) || (status & (1 << 3))) {
>> + mx27_camera_frame_done_emma(pcdev, 1, 0);
>
> "false"
>
>> }
>> - if ((status & (1 << 6)) || (status & (1 << 4)))
>> - mx27_camera_frame_done_emma(pcdev, 0);
>> - if ((status & (1 << 5)) || (status & (1 << 3)))
>> - mx27_camera_frame_done_emma(pcdev, 1);
>>
>> writel(status, pcdev->base_emma + PRP_INTRSTATUS);
>>
Don't worry, this will be fixed in v3.
--
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
[PATCH v3 1/4] media i.MX27 camera: migrate driver to videobuf2
Signed-off-by: Javier Martin
---
Changes since v2:
- Return ENOTTY for VIDIOC_CREATE_BUFS
- Check buffer size properly.
- Do not remove unfixed FIXME.
- Remove mx25 queued buffers from the queue list at release.
---
drivers/media/video/mx2_camera.c | 289 +-
1 files changed, 127 insertions(+), 162 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index ca76dd2..ae7cae6 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -3,6 +3,7 @@
*
* Copyright (C) 2008, Sascha Hauer, Pengutronix
* Copyright (C) 2010, Baruch Siach, Orex Computed Radiography
+ * Copyright (C) 2012, Javier Martin, Vista Silicon S.L.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -30,8 +31,8 @@
#include
#include
-#include
-#include
+#include
+#include
#include
#include
@@ -223,6 +224,22 @@ struct mx2_fmt_cfg {
struct mx2_prp_cfg cfg;
};
+enum mx2_buffer_state {
+ MX2_STATE_QUEUED,
+ MX2_STATE_ACTIVE,
+ MX2_STATE_DONE,
+};
+
+/* buffer for one video frame */
+struct mx2_buffer {
+ /* common v4l buffer stuff -- must be first */
+ struct vb2_buffer vb;
+ struct list_headqueue;
+ enum mx2_buffer_state state;
+
+ int bufnum;
+};
+
struct mx2_camera_dev {
struct device *dev;
struct soc_camera_host soc_host;
@@ -256,16 +273,7 @@ struct mx2_camera_dev {
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
u32 frame_count;
-};
-
-/* buffer for one video frame */
-struct mx2_buffer {
- /* common v4l buffer stuff -- must be first */
- struct videobuf_buffer vb;
-
- enum v4l2_mbus_pixelcodecode;
-
- int bufnum;
+ struct vb2_alloc_ctx*alloc_ctx;
};
static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
@@ -407,7 +415,7 @@ static irqreturn_t mx27_camera_irq(int irq_csi, void *data)
static void mx25_camera_frame_done(struct mx2_camera_dev *pcdev, int fb,
int state)
{
- struct videobuf_buffer *vb;
+ struct vb2_buffer *vb;
struct mx2_buffer *buf;
struct mx2_buffer **fb_active = fb == 1 ? &pcdev->fb1_active :
&pcdev->fb2_active;
@@ -420,25 +428,24 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
goto out;
vb = &(*fb_active)->vb;
- dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%08lx %d\n", __func__,
- vb, vb->baddr, vb->bsize);
+ dev_dbg(pcdev->dev, "%s (vb=0x%p) 0x%p %lu\n", __func__,
+ vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
- vb->state = state;
- do_gettimeofday(&vb->ts);
- vb->field_count++;
-
- wake_up(&vb->done);
+ do_gettimeofday(&vb->v4l2_buf.timestamp);
+ vb->v4l2_buf.sequence++;
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
if (list_empty(&pcdev->capture)) {
buf = NULL;
writel(0, pcdev->base_csi + fb_reg);
} else {
buf = list_entry(pcdev->capture.next, struct mx2_buffer,
- vb.queue);
+ queue);
vb = &buf->vb;
- list_del(&vb->queue);
- vb->state = VIDEOBUF_ACTIVE;
- writel(videobuf_to_dma_contig(vb), pcdev->base_csi + fb_reg);
+ list_del(&buf->queue);
+ buf->state = MX2_STATE_ACTIVE;
+ writel(vb2_dma_contig_plane_dma_addr(vb, 0),
+ pcdev->base_csi + fb_reg);
}
*fb_active = buf;
@@ -453,9 +460,9 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void *data)
u32 status = readl(pcdev->base_csi + CSISR);
if (status & CSISR_DMA_TSF_FB1_INT)
- mx25_camera_frame_done(pcdev, 1, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 1, MX2_STATE_DONE);
else if (status & CSISR_DMA_TSF_FB2_INT)
- mx25_camera_frame_done(pcdev, 2, VIDEOBUF_DONE);
+ mx25_camera_frame_done(pcdev, 2, MX2_STATE_DONE);
/* FIXME: handle CSISR_RFF_OR_INT */
@@ -467,59 +474,50 @@ static irqreturn_t mx25_camera_irq(int irq_csi, void
*data)
/*
* Videobuf operations
*/
-static int mx2_videobuf_setup(struct videobuf_queue *vq, unsigned int *count,
- unsigned int *size)
+static int mx2_videobuf_setup(struct vb2_queue *vq,
+ const struct v4l2_format *fmt,
+ unsigned int *count, unsigned int *num_planes,
+
[PATCH v3 2/4] media i.MX27 camera: add start_stream and stop_stream callbacks.
Add "start_stream" and "stop_stream" callback in order to enable
and disable the eMMa-PrP properly and save CPU usage avoiding
IRQs when the device is not streaming. This also makes the driver
return 0 as the sequence number of the first frame.
Signed-off-by: Javier Martin
---
- Make start_streaming cleaner.
---
drivers/media/video/mx2_camera.c | 100 -
1 files changed, 75 insertions(+), 25 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index ae7cae6..35ab971 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -377,7 +377,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = 0;
+ pcdev->frame_count = -1;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -656,11 +656,79 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static int mx2_start_streaming(struct vb2_queue *q, unsigned int count)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ if (mx27_camera_emma(pcdev)) {
+ unsigned long flags;
+ if (count < 2)
+ return -EINVAL;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (prp->cfg.channel == 1) {
+ writel(PRP_CNTL_CH1EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH1_LEN |
+ PRP_CNTL_CH1BYP |
+ PRP_CNTL_CH1_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ } else {
+ writel(PRP_CNTL_CH2EN |
+ PRP_CNTL_CSIEN |
+ prp->cfg.in_fmt |
+ prp->cfg.out_fmt |
+ PRP_CNTL_CH2_LEN |
+ PRP_CNTL_CH2_TSKIP(0) |
+ PRP_CNTL_IN_TSKIP(0),
+ pcdev->base_emma + PRP_CNTL);
+ }
+ spin_unlock_irqrestore(&pcdev->lock, flags);
+ }
+
+ return 0;
+}
+
+static int mx2_stop_streaming(struct vb2_queue *q)
+{
+ struct soc_camera_device *icd = soc_camera_from_vb2q(q);
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+ unsigned long flags;
+ u32 cntl;
+
+ spin_lock_irqsave(&pcdev->lock, flags);
+ if (mx27_camera_emma(pcdev)) {
+ cntl = readl(pcdev->base_emma + PRP_CNTL);
+ if (prp->cfg.channel == 1) {
+ writel(cntl & ~PRP_CNTL_CH1EN,
+ pcdev->base_emma + PRP_CNTL);
+ } else {
+ writel(cntl & ~PRP_CNTL_CH2EN,
+ pcdev->base_emma + PRP_CNTL);
+ }
+ }
+ spin_unlock_irqrestore(&pcdev->lock, flags);
+
+ return 0;
+}
+
static struct vb2_ops mx2_videobuf_ops = {
- .queue_setup= mx2_videobuf_setup,
- .buf_prepare= mx2_videobuf_prepare,
- .buf_queue = mx2_videobuf_queue,
- .buf_cleanup= mx2_videobuf_release,
+ .queue_setup = mx2_videobuf_setup,
+ .buf_prepare = mx2_videobuf_prepare,
+ .buf_queue = mx2_videobuf_queue,
+ .buf_cleanup = mx2_videobuf_release,
+ .start_streaming = mx2_start_streaming,
+ .stop_streaming = mx2_stop_streaming,
};
static int mx2_camera_init_videobuf(struct vb2_queue *q,
@@ -718,16 +786,6 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
writel(pcdev->discard_buffer_dma,
pcdev->base_emma + PRP_DEST_RGB2_PTR);
- writel(PRP_CNTL_CH1EN |
- PRP_CNTL_CSIEN |
- prp->cfg.in_fmt |
- prp->cfg.out_fmt |
- PRP_CNTL_CH1_LEN |
- PRP_CNTL_CH1BYP |
- PRP_CNTL_CH1_TSKIP(0) |
- PRP_CNTL_IN_TSKIP(0),
[PATCH v3 3/4] media i.MX27 camera: improve discard buffer handling.
The way discard buffer was previously handled lead
to possible races that made a buffer that was not
yet ready to be overwritten by new video data. This
is easily detected at 25fps just adding "#define DEBUG"
to enable the "memset" check and seeing how the image
is corrupted.
A new "discard" queue and two discard buffers have
been added to make them flow trough the pipeline
of queues and thus provide suitable event ordering.
Signed-off-by: Javier Martin
---
Changes since v2:
- Remove BUG_ON when active list is empty.
- Replace empty list checks with warnings.
---
drivers/media/video/mx2_camera.c | 280 +-
1 files changed, 153 insertions(+), 127 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 35ab971..e7ccd97 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -237,7 +237,8 @@ struct mx2_buffer {
struct list_headqueue;
enum mx2_buffer_state state;
- int bufnum;
+ int bufnum;
+ booldiscard;
};
struct mx2_camera_dev {
@@ -256,6 +257,7 @@ struct mx2_camera_dev {
struct list_headcapture;
struct list_headactive_bufs;
+ struct list_headdiscard;
spinlock_t lock;
@@ -268,6 +270,7 @@ struct mx2_camera_dev {
u32 csicr1;
+ struct mx2_buffer buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -329,6 +332,29 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
return &mx27_emma_prp_table[0];
};
+static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
+unsigned long phys, int bufnum)
+{
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ if (prp->cfg.channel == 1) {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum);
+ } else {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_Y_PTR - 0x14 * bufnum);
+ if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
+ u32 imgsize = pcdev->icd->user_height *
+ pcdev->icd->user_width;
+
+ writel(phys + imgsize, pcdev->base_emma +
+ PRP_DEST_CB_PTR - 0x14 * bufnum);
+ writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
+ PRP_DEST_CR_PTR - 0x14 * bufnum);
+ }
+ }
+}
+
static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
{
unsigned long flags;
@@ -377,7 +403,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = -1;
+ pcdev->frame_count = 0;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -397,13 +423,6 @@ static void mx2_camera_remove_device(struct
soc_camera_device *icd)
mx2_camera_deactivate(pcdev);
- if (pcdev->discard_buffer) {
- dma_free_coherent(ici->v4l2_dev.dev, pcdev->discard_size,
- pcdev->discard_buffer,
- pcdev->discard_buffer_dma);
- pcdev->discard_buffer = NULL;
- }
-
pcdev->icd = NULL;
}
@@ -640,7 +659,6 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
*/
spin_lock_irqsave(&pcdev->lock, flags);
- list_del_init(&buf->queue);
if (cpu_is_mx25() && buf->state == MX2_STATE_ACTIVE) {
if (pcdev->fb1_active == buf) {
pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
@@ -656,6 +674,34 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
+ int bytesperline)
+{
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->base_emma + PRP_SRC_FRAME_SIZE);
+ writel(prp->cfg.src_pixel,
+ pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
+ if (prp->cfg.channel == 1) {
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->base_emma + PRP_CH
[PATCH v3 4/4] media i.MX27 camera: handle overflows properly.
Signed-off-by: Javier Martin
---
Changes since v2:
- Use true and false for bool variables.
---
drivers/media/video/mx2_camera.c | 38 --
1 files changed, 16 insertions(+), 22 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index e7ccd97..09bcfe0 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1210,7 +1210,7 @@ static struct soc_camera_host_ops mx2_soc_camera_host_ops
= {
};
static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
- int bufnum)
+ int bufnum, bool err)
{
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
struct mx2_buffer *buf;
@@ -1258,7 +1258,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
list_del_init(&buf->queue);
do_gettimeofday(&vb->v4l2_buf.timestamp);
vb->v4l2_buf.sequence = pcdev->frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (err)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev->frame_count++;
@@ -1302,21 +1305,12 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
__func__);
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);
- }
- if (((status & (3 << 5)) == (3 << 5)) ||
+ buf = list_entry(pcdev->active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf->bufnum, true);
+ status &= ~(1 << 7);
+ } else if (((status & (3 << 5)) == (3 << 5)) ||
((status & (3 << 3)) == (3 << 3))) {
/*
* Both buffers have triggered, process the one we're expecting
@@ -1324,13 +1318,13 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
*/
buf = list_entry(pcdev->active_bufs.next,
struct mx2_buffer, queue);
- mx27_camera_frame_done_emma(pcdev, buf->bufnum);
+ mx27_camera_frame_done_emma(pcdev, buf->bufnum, false);
status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
+ } else if ((status & (1 << 6)) || (status & (1 << 4))) {
+ mx27_camera_frame_done_emma(pcdev, 0, false);
+ } else if ((status & (1 << 5)) || (status & (1 << 3))) {
+ mx27_camera_frame_done_emma(pcdev, 1, false);
}
- if ((status & (1 << 6)) || (status & (1 << 4)))
- mx27_camera_frame_done_emma(pcdev, 0);
- if ((status & (1 << 5)) || (status & (1 << 3)))
- mx27_camera_frame_done_emma(pcdev, 1);
writel(status, pcdev->base_emma + PRP_INTRSTATUS);
--
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
[PATCH 1/2] media: tvp5150: Add cropping support.
Signed-off-by: Javier Martin
---
drivers/media/video/tvp5150.c | 127 +++--
1 files changed, 122 insertions(+), 5 deletions(-)
diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
index c58c8d5..b1476f6 100644
--- a/drivers/media/video/tvp5150.c
+++ b/drivers/media/video/tvp5150.c
@@ -16,6 +16,13 @@
#include "tvp5150_reg.h"
+#define TVP5150_H_MAX 720
+#define TVP5150_V_MAX_525_60 480
+#define TVP5150_V_MAX_OTHERS 576
+#define TVP5150_MAX_CROP_LEFT 511
+#define TVP5150_MAX_CROP_TOP 127
+#define TVP5150_CROP_SHIFT 2
+
MODULE_DESCRIPTION("Texas Instruments TVP5150A video decoder driver");
MODULE_AUTHOR("Mauro Carvalho Chehab");
MODULE_LICENSE("GPL");
@@ -28,6 +35,7 @@ MODULE_PARM_DESC(debug, "Debug level (0-2)");
struct tvp5150 {
struct v4l2_subdev sd;
struct v4l2_ctrl_handler hdl;
+ struct v4l2_rect rect;
v4l2_std_id norm; /* Current set standard */
u32 input;
@@ -731,6 +739,13 @@ static int tvp5150_s_std(struct v4l2_subdev *sd,
v4l2_std_id std)
if (decoder->norm == std)
return 0;
+ /* Change cropping height limits */
+ if (std & V4L2_STD_525_60)
+ decoder->rect.height = TVP5150_V_MAX_525_60;
+ else
+ decoder->rect.height = TVP5150_V_MAX_OTHERS;
+
+
return tvp5150_set_std(sd, std);
}
@@ -827,11 +842,8 @@ static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
else
std = decoder->norm;
- f->width = 720;
- if (std & V4L2_STD_525_60)
- f->height = 480;
- else
- f->height = 576;
+ f->width = decoder->rect.width;
+ f->height = decoder->rect.height;
f->code = V4L2_MBUS_FMT_YUYV8_2X8;
f->field = V4L2_FIELD_SEQ_TB;
@@ -842,6 +854,99 @@ static int tvp5150_mbus_fmt(struct v4l2_subdev *sd,
return 0;
}
+static int tvp5150_s_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+{
+ struct v4l2_rect rect = a->c;
+ struct tvp5150 *decoder = to_tvp5150(sd);
+ v4l2_std_id std;
+ int hmax;
+
+ v4l2_dbg(1, debug, sd, "%s left=%d, top=%d, width=%d, height=%d\n",
+ __func__, rect.left, rect.top, rect.width, rect.height);
+
+ if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ /* tvp5150 has some special limits */
+ rect.left = clamp(rect.left, 0, TVP5150_MAX_CROP_LEFT);
+ rect.width = clamp(rect.width,
+ TVP5150_H_MAX - TVP5150_MAX_CROP_LEFT - rect.left,
+ TVP5150_H_MAX - rect.left);
+ rect.top = clamp(rect.top, 0, TVP5150_MAX_CROP_TOP);
+
+ /* Calculate height based on current standard */
+ if (decoder->norm == V4L2_STD_ALL)
+ std = tvp5150_read_std(sd);
+ else
+ std = decoder->norm;
+
+ if (std & V4L2_STD_525_60)
+ hmax = TVP5150_V_MAX_525_60;
+ else
+ hmax = TVP5150_V_MAX_OTHERS;
+
+ rect.height = clamp(rect.height,
+ hmax - TVP5150_MAX_CROP_TOP - rect.top,
+ hmax - rect.top);
+
+ tvp5150_write(sd, TVP5150_VERT_BLANKING_START, rect.top);
+ tvp5150_write(sd, TVP5150_VERT_BLANKING_STOP,
+ rect.top + rect.height - hmax);
+ tvp5150_write(sd, TVP5150_ACT_VD_CROP_ST_MSB,
+ rect.left >> TVP5150_CROP_SHIFT);
+ tvp5150_write(sd, TVP5150_ACT_VD_CROP_ST_LSB,
+ rect.left | (1 << TVP5150_CROP_SHIFT));
+ tvp5150_write(sd, TVP5150_ACT_VD_CROP_STP_MSB,
+ (rect.left + rect.width - TVP5150_MAX_CROP_LEFT) >>
+ TVP5150_CROP_SHIFT);
+ tvp5150_write(sd, TVP5150_ACT_VD_CROP_STP_LSB,
+ rect.left + rect.width - TVP5150_MAX_CROP_LEFT);
+
+ decoder->rect = rect;
+
+ return 0;
+}
+
+static int tvp5150_g_crop(struct v4l2_subdev *sd, struct v4l2_crop *a)
+{
+ struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+
+ a->c= decoder->rect;
+ a->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+
+ return 0;
+}
+
+static int tvp5150_cropcap(struct v4l2_subdev *sd, struct v4l2_cropcap *a)
+{
+ struct tvp5150 *decoder = container_of(sd, struct tvp5150, sd);
+ v4l2_std_id std;
+
+ if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+ return -EINVAL;
+
+ a->bounds.left = 0;
+ a->bounds.top = 0;
+ a->bounds.width = TVP5150_H_MAX;
+
+ /* Calculate height based on current standard */
+ if (decoder->norm == V4L2_STD_ALL)
+ std = tvp5150_read_std(sd);
+ else
+
[PATCH 2/2] media: tvp5150: support g_mbus_fmt callback.
Signed-off-by: Javier Martin
---
drivers/media/video/tvp5150.c |1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c
index b1476f6..e292c46 100644
--- a/drivers/media/video/tvp5150.c
+++ b/drivers/media/video/tvp5150.c
@@ -1102,6 +1102,7 @@ static const struct v4l2_subdev_video_ops
tvp5150_video_ops = {
.enum_mbus_fmt = tvp5150_enum_mbus_fmt,
.s_mbus_fmt = tvp5150_mbus_fmt,
.try_mbus_fmt = tvp5150_mbus_fmt,
+ .g_mbus_fmt = tvp5150_mbus_fmt,
.s_crop = tvp5150_s_crop,
.g_crop = tvp5150_g_crop,
.cropcap = tvp5150_cropcap,
--
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
Re: OV2640 and iMX25PDK - help needed
Hi, On 2 February 2012 15:01, Fabio Estevam wrote: > On 2/1/12, Guennadi Liakhovetski wrote: >> Hello Gonzalo >> >> On Tue, 31 Jan 2012, Fernandez Gonzalo wrote: >> >>> Hi all, >>> >>> I've been working for a while with an iMX25PDK using the BSP provided by >>> Freescale (L2.6.31). The camera driver (V4L2-int) and examples do the >>> job quite well but I need to move my design to a more recent kernel. >>> I've been extensively googling but haven't found any info/examples about >>> how to run the mx2_camera driver in the i.MX25PDK. I'm stuck at this, >>> could someone point me in the right direction? Thank you in advance... >> >> i.MX25PDK is supported in the mainline kernel >> (arch/arm/mach-imx/mach-mx25_3ds.c), but it doesn't attach any cameras. >> Unfortunately, I also don't currently see any i.MX2x platforms in the >> mainline with cameras, so, you have to begin by looking at >> arch/arm/plat-mxc/include/mach/mx2_cam.h, at >> arch/arm/plat-mxc/devices/platform-mx2-camera.c for the >> imx27_add_mx2_camera() function and maybe some i.MX3x or i.MX1 examples. > > Javier has been doing a lot of work on mx2-camera lately. > > Javier, > > Is mach-imx27_visstrim_m10 board connected to a CMOS camera? Do you > have patches for adding camera support to mach-imx27_visstrim_m10? visstrim_m10 is connected to a tvp5150 but it uses the same interface as a CMOS sensor. Let me find some time to send a patch that I have pending in my queue. Then it can be used by Gonzalo as a reference. 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
[dmaengine] [Q] jiffies value does not increase in dma_sync_wait()
Hi, I have a Visstrim_M10 board, based on i.MX27, and I'm developing a v4l2 driver for deinterlacing video frames. Whenever I start a new dma transfer I call the function "dma_wait_for_async_tx()" which internally ends up calling "dma_sync_wait()": http://lxr.linux.no/#linux+v3.2.2/drivers/dma/dmaengine.c#L255 In this function, there is a "do while" loop, which checks for dma completion, with a timeout. However, when the system is too loaded this function enters this "do while" loop and never gets out of it, blocking the system. I've introduced a couple of printk() to check why this timeout is not triggered and I've found that the value of jiffies does not increase between loop iterations (i. e. it's like time didn't advance). Does anyobody know what reasons could make jiffies not being updated? 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: [dmaengine] [Q] jiffies value does not increase in dma_sync_wait()
Hi Russell, On 3 February 2012 10:10, Russell King - ARM Linux wrote: > On Fri, Feb 03, 2012 at 09:37:48AM +0100, javier Martin wrote: >> I've introduced a couple of printk() to check why this timeout is not >> triggered and I've found that the value of jiffies does not increase >> between loop iterations (i. e. it's like time didn't advance). >> >> Does anyobody know what reasons could make jiffies not being updated? > > Are interrupts disabled? Apparently not but, how could I check it for sure? Is "irqs_disabled()" suitable for that purpose? http://lxr.linux.no/#linux+v3.2.2/include/linux/irqflags.h#L120 -- 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: [dmaengine] [Q] jiffies value does not increase in dma_sync_wait()
On 3 February 2012 10:24, Russell King - ARM Linux wrote: > On Fri, Feb 03, 2012 at 10:22:00AM +0100, javier Martin wrote: >> Hi Russell, >> >> On 3 February 2012 10:10, Russell King - ARM Linux >> wrote: >> > On Fri, Feb 03, 2012 at 09:37:48AM +0100, javier Martin wrote: >> >> I've introduced a couple of printk() to check why this timeout is not >> >> triggered and I've found that the value of jiffies does not increase >> >> between loop iterations (i. e. it's like time didn't advance). >> >> >> >> Does anyobody know what reasons could make jiffies not being updated? >> > >> > Are interrupts disabled? >> >> Apparently not but, how could I check it for sure? Is >> "irqs_disabled()" suitable for that purpose? > > Yes. "irqs_disabled()" returns 0 in every iteration of the loop. So I guess this means IRQs are properly enabled, but jiffies keeps being fixed. -- 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: videobuf2 porting
Hi Prabhakar, On 7 February 2012 08:16, Prabhakar Lad wrote: > Hi folks, > > I was trying to port a existing driver to videobuf2 framework, I had a > hurdle in between > the driver I was trying to port supports user pointer, How does > videobuf2 support this > thing ? If the driver you are trying to port supports old videobuf framework this patch might be useful for you: http://patchwork.linuxtv.org/patch/9719/ -- 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 v3 3/4] media i.MX27 camera: improve discard buffer handling.
Hi Guennadi,
thank you for your attention.
On 6 February 2012 19:33, Guennadi Liakhovetski wrote:
> Hi Javier
>
> Thanks for the update! Let's see, whether this one can be improved a bit
> more.
>
> On Mon, 30 Jan 2012, Javier Martin wrote:
>
>> The way discard buffer was previously handled lead
>> to possible races that made a buffer that was not
>> yet ready to be overwritten by new video data. This
>> is easily detected at 25fps just adding "#define DEBUG"
>> to enable the "memset" check and seeing how the image
>> is corrupted.
>>
>> A new "discard" queue and two discard buffers have
>> been added to make them flow trough the pipeline
>> of queues and thus provide suitable event ordering.
>>
>> Signed-off-by: Javier Martin
>> ---
>> Changes since v2:
>> - Remove BUG_ON when active list is empty.
>> - Replace empty list checks with warnings.
>
> I think, the best would be to warn and bail out, instead of implicitly
> crashing.
>
>>
>> ---
>> drivers/media/video/mx2_camera.c | 280
>> +-
>> 1 files changed, 153 insertions(+), 127 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 35ab971..e7ccd97 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>
> [snip]
>
>> @@ -706,8 +806,9 @@ static int mx2_stop_streaming(struct vb2_queue *q)
>> unsigned long flags;
>> u32 cntl;
>>
>> - spin_lock_irqsave(&pcdev->lock, flags);
>> if (mx27_camera_emma(pcdev)) {
>> + spin_lock_irqsave(&pcdev->lock, flags);
>> +
>> cntl = readl(pcdev->base_emma + PRP_CNTL);
>> if (prp->cfg.channel == 1) {
>> writel(cntl & ~PRP_CNTL_CH1EN,
>> @@ -716,8 +817,18 @@ static int mx2_stop_streaming(struct vb2_queue *q)
>> writel(cntl & ~PRP_CNTL_CH2EN,
>> pcdev->base_emma + PRP_CNTL);
>> }
>> + INIT_LIST_HEAD(&pcdev->capture);
>> + INIT_LIST_HEAD(&pcdev->active_bufs);
>> + INIT_LIST_HEAD(&pcdev->discard);
>> +
>> + spin_unlock_irqrestore(&pcdev->lock, flags);
>> +
>> + dma_free_coherent(ici->v4l2_dev.dev,
>> + pcdev->discard_size, pcdev->discard_buffer,
>> + pcdev->discard_buffer_dma);
>> + pcdev->discard_buffer = NULL;
>
> AFAICS, the IRQ handler runs without taking any locks, so, there's a
> theoretical SMP race here with using the discard buffers from the ISR. So,
> I think, you'd have to add some locking to the ISR and here do something
> like
>
> + x = pcdev->discard_buffer;
> + pcdev->discard_buffer = NULL;
> +
> + spin_unlock_irqrestore(&pcdev->lock, flags);
> +
> + dma_free_coherent(ici->v4l2_dev.dev,
> + pcdev->discard_size, x,
> + pcdev->discard_buffer_dma);
Hmm, you are definitely right. I have to protect access to
discard_buffer too. Good you noticed.
>
>> }
>> - spin_unlock_irqrestore(&pcdev->lock, flags);
>> +
>
> You're adding an empty line here.
Sorry, I'll fix it.
>>
>> return 0;
>> }
>
> [snip]
>
>> @@ -1179,18 +1212,23 @@ static struct soc_camera_host_ops
>> mx2_soc_camera_host_ops = {
>> static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
>> int bufnum)
>> {
>
> This function is called from the ISR, so, I presume, you'll have to
> spin_lock() somewhere here.
Yes, otherwise I can have a lot of possible races here.
>> - u32 imgsize = pcdev->icd->user_height * pcdev->icd->user_width;
>> struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>> struct mx2_buffer *buf;
>> struct vb2_buffer *vb;
>> unsigned long phys;
>>
>> - if (!list_empty(&pcdev->active_bufs)) {
>> - buf = list_entry(pcdev->active_bufs.next,
>> - struct mx2_buffer, queue);
>> + buf = list_entry(pcdev->active_bufs.next,
>> + struct mx2_buffer, queue);
>>
>> - BUG_ON(buf->bufnum != bufnum);
>> + BUG_ON(buf->bufnum != bufnum);
>>
>> + if (buf-&
Re: [PATCH v3 3/4] media i.MX27 camera: improve discard buffer handling.
On 7 February 2012 09:22, Guennadi Liakhovetski wrote: > Hi Javier > > On Tue, 7 Feb 2012, javier Martin wrote: > > [snip] > >> Guennadi, before I send v4: >> Do you agree with the other patches 1, 2 and 4? > > Yes, this time I haven't found anything their to complain about:-) So, > feel free to just send a v4 of this patch and then your proposed follow-up > fixes / improvements. Great! On my way. -- 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
[PATCH v4 3/4] media i.MX27 camera: improve discard buffer handling.
The way discard buffer was previously handled lead
to possible races that made a buffer that was not
yet ready to be overwritten by new video data. This
is easily detected at 25fps just adding "#define DEBUG"
to enable the "memset" check and seeing how the image
is corrupted.
A new "discard" queue and two discard buffers have
been added to make them flow trough the pipeline
of queues and thus provide suitable event ordering.
Signed-off-by: Javier Martin
---
- Add proper locking to avoid races between IRQ and stop callbacks.
- If a list is unexpectedly empty want the user and bail out.
---
drivers/media/video/mx2_camera.c | 287 +-
1 files changed, 161 insertions(+), 126 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 35ab971..3880d24 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -237,7 +237,8 @@ struct mx2_buffer {
struct list_headqueue;
enum mx2_buffer_state state;
- int bufnum;
+ int bufnum;
+ booldiscard;
};
struct mx2_camera_dev {
@@ -256,6 +257,7 @@ struct mx2_camera_dev {
struct list_headcapture;
struct list_headactive_bufs;
+ struct list_headdiscard;
spinlock_t lock;
@@ -268,6 +270,7 @@ struct mx2_camera_dev {
u32 csicr1;
+ struct mx2_buffer buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -329,6 +332,29 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
return &mx27_emma_prp_table[0];
};
+static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
+unsigned long phys, int bufnum)
+{
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ if (prp->cfg.channel == 1) {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum);
+ } else {
+ writel(phys, pcdev->base_emma +
+ PRP_DEST_Y_PTR - 0x14 * bufnum);
+ if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
+ u32 imgsize = pcdev->icd->user_height *
+ pcdev->icd->user_width;
+
+ writel(phys + imgsize, pcdev->base_emma +
+ PRP_DEST_CB_PTR - 0x14 * bufnum);
+ writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
+ PRP_DEST_CR_PTR - 0x14 * bufnum);
+ }
+ }
+}
+
static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
{
unsigned long flags;
@@ -377,7 +403,7 @@ static int mx2_camera_add_device(struct soc_camera_device
*icd)
writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
pcdev->icd = icd;
- pcdev->frame_count = -1;
+ pcdev->frame_count = 0;
dev_info(icd->parent, "Camera driver attached to camera %d\n",
icd->devnum);
@@ -397,13 +423,6 @@ static void mx2_camera_remove_device(struct
soc_camera_device *icd)
mx2_camera_deactivate(pcdev);
- if (pcdev->discard_buffer) {
- dma_free_coherent(ici->v4l2_dev.dev, pcdev->discard_size,
- pcdev->discard_buffer,
- pcdev->discard_buffer_dma);
- pcdev->discard_buffer = NULL;
- }
-
pcdev->icd = NULL;
}
@@ -640,7 +659,6 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
*/
spin_lock_irqsave(&pcdev->lock, flags);
- list_del_init(&buf->queue);
if (cpu_is_mx25() && buf->state == MX2_STATE_ACTIVE) {
if (pcdev->fb1_active == buf) {
pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
@@ -656,6 +674,34 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
spin_unlock_irqrestore(&pcdev->lock, flags);
}
+static void mx27_camera_emma_buf_init(struct soc_camera_device *icd,
+ int bytesperline)
+{
+ struct soc_camera_host *ici =
+ to_soc_camera_host(icd->parent);
+ struct mx2_camera_dev *pcdev = ici->priv;
+ struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+
+ writel((icd->user_width << 16) | icd->user_height,
+ pcdev->base_emma + PRP_SRC_FRAME_SIZE);
+ writel(prp->cfg.src_pixel,
+ pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
+ if (prp->cfg.channel == 1) {
+ writel((icd->user_width << 16) | icd->user_height,
+
Re: [PATCH v4 3/4] media i.MX27 camera: improve discard buffer handling.
Hi Guennadi,
I understand you are probably quite busy right now but it would be
great if you could ack this patch. The sooner you merge it the sooner
I will start working on the cleanup series. I've got some time on my
hands now.
Thank you.
On 7 February 2012 11:14, Javier Martin wrote:
> The way discard buffer was previously handled lead
> to possible races that made a buffer that was not
> yet ready to be overwritten by new video data. This
> is easily detected at 25fps just adding "#define DEBUG"
> to enable the "memset" check and seeing how the image
> is corrupted.
>
> A new "discard" queue and two discard buffers have
> been added to make them flow trough the pipeline
> of queues and thus provide suitable event ordering.
>
> Signed-off-by: Javier Martin
> ---
> - Add proper locking to avoid races between IRQ and stop callbacks.
> - If a list is unexpectedly empty want the user and bail out.
>
> ---
> drivers/media/video/mx2_camera.c | 287
> +-
> 1 files changed, 161 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/media/video/mx2_camera.c
> b/drivers/media/video/mx2_camera.c
> index 35ab971..3880d24 100644
> --- a/drivers/media/video/mx2_camera.c
> +++ b/drivers/media/video/mx2_camera.c
> @@ -237,7 +237,8 @@ struct mx2_buffer {
> struct list_head queue;
> enum mx2_buffer_state state;
>
> - int bufnum;
> + int bufnum;
> + bool discard;
> };
>
> struct mx2_camera_dev {
> @@ -256,6 +257,7 @@ struct mx2_camera_dev {
>
> struct list_head capture;
> struct list_head active_bufs;
> + struct list_head discard;
>
> spinlock_t lock;
>
> @@ -268,6 +270,7 @@ struct mx2_camera_dev {
>
> u32 csicr1;
>
> + struct mx2_buffer buf_discard[2];
> void *discard_buffer;
> dma_addr_t discard_buffer_dma;
> size_t discard_size;
> @@ -329,6 +332,29 @@ static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
> return &mx27_emma_prp_table[0];
> };
>
> +static void mx27_update_emma_buf(struct mx2_camera_dev *pcdev,
> + unsigned long phys, int bufnum)
> +{
> + struct mx2_fmt_cfg *prp = pcdev->emma_prp;
> +
> + if (prp->cfg.channel == 1) {
> + writel(phys, pcdev->base_emma +
> + PRP_DEST_RGB1_PTR + 4 * bufnum);
> + } else {
> + writel(phys, pcdev->base_emma +
> + PRP_DEST_Y_PTR - 0x14 * bufnum);
> + if (prp->out_fmt == V4L2_PIX_FMT_YUV420) {
> + u32 imgsize = pcdev->icd->user_height *
> + pcdev->icd->user_width;
> +
> + writel(phys + imgsize, pcdev->base_emma +
> + PRP_DEST_CB_PTR - 0x14 * bufnum);
> + writel(phys + ((5 * imgsize) / 4), pcdev->base_emma +
> + PRP_DEST_CR_PTR - 0x14 * bufnum);
> + }
> + }
> +}
> +
> static void mx2_camera_deactivate(struct mx2_camera_dev *pcdev)
> {
> unsigned long flags;
> @@ -377,7 +403,7 @@ static int mx2_camera_add_device(struct soc_camera_device
> *icd)
> writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
>
> pcdev->icd = icd;
> - pcdev->frame_count = -1;
> + pcdev->frame_count = 0;
>
> dev_info(icd->parent, "Camera driver attached to camera %d\n",
> icd->devnum);
> @@ -397,13 +423,6 @@ static void mx2_camera_remove_device(struct
> soc_camera_device *icd)
>
> mx2_camera_deactivate(pcdev);
>
> - if (pcdev->discard_buffer) {
> - dma_free_coherent(ici->v4l2_dev.dev, pcdev->discard_size,
> - pcdev->discard_buffer,
> - pcdev->discard_buffer_dma);
> - pcdev->discard_buffer = NULL;
> - }
> -
> pcdev->icd = NULL;
> }
>
> @@ -640,7 +659,6 @@ static void mx2_videobuf_release(struct vb2_buffer *vb)
> */
>
> spin_lock_irqsave(&pcdev->lock, flags);
> - list_del_init(&buf->queue);
> if (cpu_is_mx25() && buf->state == MX2_STATE_ACTIVE) {
> if (pcdev->fb1_active == buf) {
> pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
> @@
Re: [PATCH v4 3/4] media i.MX27 camera: improve discard buffer handling.
Hi Guennadi,
On 9 February 2012 23:36, Guennadi Liakhovetski wrote:
> Hi Javier
>
> On Thu, 9 Feb 2012, javier Martin wrote:
>
>> Hi Guennadi,
>> I understand you are probably quite busy right now but it would be
>> great if you could ack this patch. The sooner you merge it the sooner
>> I will start working on the cleanup series. I've got some time on my
>> hands now.
>
> Yes, I can take this version, at the same time, I have a couple of
> comments, that you might find useful to address in a clean-up patch;-) Or
> just leave them as they are...
Of course,
I'm the most interested person on this driver being as better as possible.
> [anip]
>
>
>> > @@ -1274,6 +1298,15 @@ static irqreturn_t mx27_camera_emma_irq(int
>> > irq_emma, void *data)
>> >struct mx2_camera_dev *pcdev = data;
>> >unsigned int status = readl(pcdev->base_emma + PRP_INTRSTATUS);
>> >struct mx2_buffer *buf;
>> > + unsigned long flags;
>> > +
>> > + spin_lock_irqsave(&pcdev->lock, flags);
>
> It wasn't an accident, that I wrote "spin_lock()" - without the "_irqsave"
> part. You are in an ISR here, and this is the only IRQ, that your driver
> has to protect against, so, here, I think, you don't have to block other
> IRQs.
Ok,
>> > +
>> > + if (list_empty(&pcdev->active_bufs)) {
>> > + dev_warn(pcdev->dev, "%s: called while active list is
>> > empty\n",
>> > + __func__);
>> > + goto irq_ok;
>
> This means, you return IRQ_HANDLED here without even checking whether any
> of your status bits are actually set. So, if you get an interrupt here
> with an empty list, it might indeed be the case of a shared IRQ, in which
> case you'd have to return IRQ_NONE.
Got it.
>> > + }
>> >
>> >if (status & (1 << 7)) { /* overflow */
>> >u32 cntl;
>
> As I said - we can keep this version, but maybe you'll like to improve at
> least the latter of the above two snippets.
I'd rather you merge this as it is, because it really fixes a driver
which is currently buggy. I'll send a clean up series adressing the
following issues next week:
1. Eliminate the unwanted "goto".
2. Use list_first_entry() macro.
3. Use spin_lock() in ISR.
4. Return IRQ_NONE if list is empty and no status bit is set.
5. Integrate discard buffers in a more efficient way.
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
[PATCH 0/6] media: i.MX27 camera: Clean up series.
Hi Guennadi, This is the clean up series I promised to send this week. This has to be applied on top of my previous patches. These are already discussed issues so I don't think you have any concerns with them. While I wait for your confirmation, I'm going to prepare a new patch in order to provide video resizing support. [PATCH 1/6] media: i.MX27 camera: Remove goto from mx2_videobuf_queue(). [PATCH 2/6] media: i.MX27 camera: Use list_first_entry() whenever possible. [PATCH 3/6] media: i.MX27 camera: Use spin_lock() inside the IRQ handler. [PATCH 4/6] media: i.MX27 camera: return IRQ_NONE if no IRQ status bit is set. [PATCH 5/6] media: i.MX27 camera: fix compilation warning. [PATCH 6/6] media: i.MX27 camera: more efficient discard buffer handling. -- 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 1/6] media: i.MX27 camera: Remove goto from mx2_videobuf_queue().
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index e70d26f..1f046a3 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -580,9 +580,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
buf->state = MX2_STATE_QUEUED;
list_add_tail(&buf->queue, &pcdev->capture);
- if (mx27_camera_emma(pcdev)) {
- goto out;
- } else { /* cpu_is_mx25() */
+ if (cpu_is_mx25()) {
u32 csicr3, dma_inten = 0;
if (pcdev->fb1_active == NULL) {
@@ -618,7 +616,6 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
}
}
-out:
spin_unlock_irqrestore(&pcdev->lock, flags);
}
--
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
[PATCH 2/6] media: i.MX27 camera: Use list_first_entry() whenever possible.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 28 +---
1 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 1f046a3..13be305 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -458,7 +458,7 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
buf = NULL;
writel(0, pcdev->base_csi + fb_reg);
} else {
- buf = list_entry(pcdev->capture.next, struct mx2_buffer,
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
queue);
vb = &buf->vb;
list_del(&buf->queue);
@@ -718,8 +718,8 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
spin_lock_irqsave(&pcdev->lock, flags);
- buf = list_entry(pcdev->capture.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
+ queue);
buf->bufnum = 0;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
@@ -728,8 +728,8 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
mx27_update_emma_buf(pcdev, phys, buf->bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
- buf = list_entry(pcdev->capture.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
+ queue);
buf->bufnum = 1;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
@@ -1215,8 +1215,7 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
struct vb2_buffer *vb;
unsigned long phys;
- buf = list_entry(pcdev->active_bufs.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->active_bufs, struct mx2_buffer, queue);
BUG_ON(buf->bufnum != bufnum);
@@ -1270,8 +1269,8 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
return;
}
- buf = list_entry(pcdev->discard.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->discard, struct mx2_buffer,
+ queue);
buf->bufnum = bufnum;
list_move_tail(pcdev->discard.next, &pcdev->active_bufs);
@@ -1279,8 +1278,7 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
return;
}
- buf = list_entry(pcdev->capture.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer, queue);
buf->bufnum = bufnum;
@@ -1309,8 +1307,8 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
}
if (status & (1 << 7)) { /* overflow */
- buf = list_entry(pcdev->active_bufs.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->active_bufs, struct mx2_buffer,
+ queue);
mx27_camera_frame_done_emma(pcdev,
buf->bufnum, true);
status &= ~(1 << 7);
@@ -1320,8 +1318,8 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
* Both buffers have triggered, process the one we're expecting
* to first
*/
- buf = list_entry(pcdev->active_bufs.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->active_bufs, struct mx2_buffer,
+ queue);
mx27_camera_frame_done_emma(pcdev, buf->bufnum, false);
status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
} else if ((status & (1 << 6)) || (status & (1 << 4))) {
--
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
[PATCH 4/6] media: i.MX27 camera: return IRQ_NONE if no IRQ status bit is set.
If active_bufs() list is empty and no IRQ status bit is set
we are probably dealing with a share IRQ. Return IRQ_NONE in
this case.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c |7 +--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 34b43a4..d9028f1 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1302,7 +1302,11 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
if (list_empty(&pcdev->active_bufs)) {
dev_warn(pcdev->dev, "%s: called while active list is empty\n",
__func__);
- goto irq_ok;
+
+ if (!status) {
+ spin_unlock(&pcdev->lock);
+ return IRQ_NONE;
+ }
}
if (status & (1 << 7)) { /* overflow */
@@ -1327,7 +1331,6 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
mx27_camera_frame_done_emma(pcdev, 1, false);
}
-irq_ok:
spin_unlock(&pcdev->lock);
writel(status, pcdev->base_emma + PRP_INTRSTATUS);
--
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
[PATCH 3/6] media: i.MX27 camera: Use spin_lock() inside the IRQ handler.
We don't need to use spin_lock_irqsave() since there are not
any other IRQs that can race with this ISR.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c |5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 13be305..34b43a4 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1296,9 +1296,8 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
struct mx2_camera_dev *pcdev = data;
unsigned int status = readl(pcdev->base_emma + PRP_INTRSTATUS);
struct mx2_buffer *buf;
- unsigned long flags;
- spin_lock_irqsave(&pcdev->lock, flags);
+ spin_lock(&pcdev->lock);
if (list_empty(&pcdev->active_bufs)) {
dev_warn(pcdev->dev, "%s: called while active list is empty\n",
@@ -1329,7 +1328,7 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
}
irq_ok:
- spin_unlock_irqrestore(&pcdev->lock, flags);
+ spin_unlock(&pcdev->lock);
writel(status, pcdev->base_emma + PRP_INTRSTATUS);
return IRQ_HANDLED;
--
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
[PATCH 5/6] media: i.MX27 camera: fix compilation warning.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index d9028f1..8ccdb4a 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1210,7 +1210,9 @@ static struct soc_camera_host_ops mx2_soc_camera_host_ops
= {
static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
int bufnum, bool err)
{
+#ifdef DEBUG
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+#endif
struct mx2_buffer *buf;
struct vb2_buffer *vb;
unsigned long phys;
@@ -1232,18 +1234,16 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
if (prp->cfg.channel == 1) {
if (readl(pcdev->base_emma + PRP_DEST_RGB1_PTR +
4 * bufnum) != phys) {
- dev_err(pcdev->dev, "%p != %p\n", phys,
- readl(pcdev->base_emma +
- PRP_DEST_RGB1_PTR +
- 4 * bufnum));
+ dev_err(pcdev->dev, "%p != %p\n", (void *)phys,
+ (void *)readl(pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum));
}
} else {
if (readl(pcdev->base_emma + PRP_DEST_Y_PTR -
0x14 * bufnum) != phys) {
- dev_err(pcdev->dev, "%p != %p\n", phys,
- readl(pcdev->base_emma +
- PRP_DEST_Y_PTR -
- 0x14 * bufnum));
+ dev_err(pcdev->dev, "%p != %p\n", (void *)phys,
+ (void *)readl(pcdev->base_emma +
+ PRP_DEST_Y_PTR - 0x14 * bufnum));
}
}
#endif
--
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
[PATCH 6/6] media: i.MX27 camera: more efficient discard buffer handling.
Some elements of 'mx2_buffer' are grouped together in another
auxiliary structure. This way we don't need to have unused
'vb2_buffer' structures for both discard buffers.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 77 ++
1 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 8ccdb4a..de0a19c 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -230,15 +230,18 @@ enum mx2_buffer_state {
MX2_STATE_DONE,
};
+struct mx2_buf_internal {
+ struct list_headqueue;
+ int bufnum;
+ booldiscard;
+};
+
/* buffer for one video frame */
struct mx2_buffer {
/* common v4l buffer stuff -- must be first */
struct vb2_buffer vb;
- struct list_headqueue;
enum mx2_buffer_state state;
-
- int bufnum;
- booldiscard;
+ struct mx2_buf_internal internal;
};
struct mx2_camera_dev {
@@ -270,7 +273,7 @@ struct mx2_camera_dev {
u32 csicr1;
- struct mx2_buffer buf_discard[2];
+ struct mx2_buf_internal buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -279,6 +282,11 @@ struct mx2_camera_dev {
struct vb2_alloc_ctx*alloc_ctx;
};
+static struct mx2_buffer *mx2_ibuf_to_buf(struct mx2_buf_internal *int_buf)
+{
+ return container_of(int_buf, struct mx2_buffer, internal);
+}
+
static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
/*
* This is a generic configuration which is valid for most
@@ -459,9 +467,9 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
writel(0, pcdev->base_csi + fb_reg);
} else {
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
+ internal.queue);
vb = &buf->vb;
- list_del(&buf->queue);
+ list_del(&buf->internal.queue);
buf->state = MX2_STATE_ACTIVE;
writel(vb2_dma_contig_plane_dma_addr(vb, 0),
pcdev->base_csi + fb_reg);
@@ -578,7 +586,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
spin_lock_irqsave(&pcdev->lock, flags);
buf->state = MX2_STATE_QUEUED;
- list_add_tail(&buf->queue, &pcdev->capture);
+ list_add_tail(&buf->internal.queue, &pcdev->capture);
if (cpu_is_mx25()) {
u32 csicr3, dma_inten = 0;
@@ -596,7 +604,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
}
if (dma_inten) {
- list_del(&buf->queue);
+ list_del(&buf->internal.queue);
buf->state = MX2_STATE_ACTIVE;
csicr3 = readl(pcdev->base_csi + CSICR3);
@@ -719,23 +727,23 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
spin_lock_irqsave(&pcdev->lock, flags);
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
- buf->bufnum = 0;
+ internal.queue);
+ buf->internal.bufnum = 0;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
phys = vb2_dma_contig_plane_dma_addr(vb, 0);
- mx27_update_emma_buf(pcdev, phys, buf->bufnum);
+ mx27_update_emma_buf(pcdev, phys, buf->internal.bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
- buf->bufnum = 1;
+ internal.queue);
+ buf->internal.bufnum = 1;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
phys = vb2_dma_contig_plane_dma_addr(vb, 0);
- mx27_update_emma_buf(pcdev, phys, buf->bufnum);
+ mx27_update_emma_buf(pcdev, phys, buf->internal.bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
bytesperline = soc_mbus_bytes_per_line(icd->user_width,
@@ -1213,21 +1221,25 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
#ifdef DEBUG
struct mx2_fmt_cfg *prp =
[PATCH v2 5/6] media: i.MX27 camera: fix compilation warning.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 16
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index d9028f1..06017a0 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1210,7 +1210,9 @@ static struct soc_camera_host_ops mx2_soc_camera_host_ops
= {
static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
int bufnum, bool err)
{
+#ifdef DEBUG
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
+#endif
struct mx2_buffer *buf;
struct vb2_buffer *vb;
unsigned long phys;
@@ -1232,18 +1234,16 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
if (prp->cfg.channel == 1) {
if (readl(pcdev->base_emma + PRP_DEST_RGB1_PTR +
4 * bufnum) != phys) {
- dev_err(pcdev->dev, "%p != %p\n", phys,
- readl(pcdev->base_emma +
- PRP_DEST_RGB1_PTR +
- 4 * bufnum));
+ dev_err(pcdev->dev, "%lx != %x\n", phys,
+ readl(pcdev->base_emma +
+ PRP_DEST_RGB1_PTR + 4 * bufnum));
}
} else {
if (readl(pcdev->base_emma + PRP_DEST_Y_PTR -
0x14 * bufnum) != phys) {
- dev_err(pcdev->dev, "%p != %p\n", phys,
- readl(pcdev->base_emma +
- PRP_DEST_Y_PTR -
- 0x14 * bufnum));
+ dev_err(pcdev->dev, "%lx != %x\n", phys,
+ readl(pcdev->base_emma +
+ PRP_DEST_Y_PTR - 0x14 * bufnum));
}
}
#endif
--
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
Re: [PATCH 5/6] media: i.MX27 camera: fix compilation warning.
On 13 February 2012 15:10, Guennadi Liakhovetski wrote:
> Hi Javier
>
> On Mon, 13 Feb 2012, Javier Martin wrote:
>
>>
>> Signed-off-by: Javier Martin
>> ---
>> drivers/media/video/mx2_camera.c | 16
>> 1 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index d9028f1..8ccdb4a 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -1210,7 +1210,9 @@ static struct soc_camera_host_ops
>> mx2_soc_camera_host_ops = {
>> static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
>> int bufnum, bool err)
>> {
>> +#ifdef DEBUG
>> struct mx2_fmt_cfg *prp = pcdev->emma_prp;
>> +#endif
>> struct mx2_buffer *buf;
>> struct vb2_buffer *vb;
>> unsigned long phys;
>> @@ -1232,18 +1234,16 @@ static void mx27_camera_frame_done_emma(struct
>> mx2_camera_dev *pcdev,
>> if (prp->cfg.channel == 1) {
>> if (readl(pcdev->base_emma + PRP_DEST_RGB1_PTR +
>> 4 * bufnum) != phys) {
>> - dev_err(pcdev->dev, "%p != %p\n", phys,
>> - readl(pcdev->base_emma +
>> - PRP_DEST_RGB1_PTR +
>> - 4 * bufnum));
>> + dev_err(pcdev->dev, "%p != %p\n", (void *)phys,
>> + (void *)readl(pcdev->base_emma +
>> + PRP_DEST_RGB1_PTR + 4 * bufnum));
>> }
>> } else {
>> if (readl(pcdev->base_emma + PRP_DEST_Y_PTR -
>> 0x14 * bufnum) != phys) {
>> - dev_err(pcdev->dev, "%p != %p\n", phys,
>> - readl(pcdev->base_emma +
>> - PRP_DEST_Y_PTR -
>> - 0x14 * bufnum));
>> + dev_err(pcdev->dev, "%p != %p\n", (void *)phys,
>> + (void *)readl(pcdev->base_emma +
>> + PRP_DEST_Y_PTR - 0x14 * bufnum));
>
> I think, just using %lx would be better.
Fixed.
Please, see v2.
--
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
[PATCH] media: i.MX27 camera: Add resizing support.
If the attached video sensor cannot provide the
requested image size, try to use resizing engine
included in the eMMa-PrP IP.
This patch supports both averaging and bilinear
algorithms.
Signed-off-by: Javier Martin
---
drivers/media/video/mx2_camera.c | 251 +-
1 files changed, 249 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 9e84368..6516ee4 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -204,10 +205,25 @@
#define PRP_INTR_LBOVF (1 << 7)
#define PRP_INTR_CH2OVF(1 << 8)
+/* Resizing registers */
+#define PRP_RZ_VALID_TBL_LEN(x)((x) << 24)
+#define PRP_RZ_VALID_BILINEAR (1 << 31)
+
#define mx27_camera_emma(pcdev)(cpu_is_mx27() && pcdev->use_emma)
#define MAX_VIDEO_MEM 16
+#define RESIZE_NUM_MIN 1
+#define RESIZE_NUM_MAX 20
+#define BC_COEF3
+#define SZ_COEF(1 << BC_COEF)
+
+#define RESIZE_DIR_H 0
+#define RESIZE_DIR_V 1
+
+#define RESIZE_ALGO_BILINEAR 0
+#define RESIZE_ALGO_AVERAGING 1
+
struct mx2_prp_cfg {
int channel;
u32 in_fmt;
@@ -217,6 +233,13 @@ struct mx2_prp_cfg {
u32 irq_flags;
};
+/* prp resizing parameters */
+struct emma_prp_resize {
+ int algo; /* type of algorithm used */
+ int len; /* number of coefficients */
+ unsigned char s[RESIZE_NUM_MAX]; /* table of coefficients */
+};
+
/* prp configuration for a client-host fmt pair */
struct mx2_fmt_cfg {
enum v4l2_mbus_pixelcodein_fmt;
@@ -278,6 +301,8 @@ struct mx2_camera_dev {
dma_addr_t discard_buffer_dma;
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
+ struct emma_prp_resize resizing[2];
+ unsigned ints_width, s_height;
u32 frame_count;
struct vb2_alloc_ctx*alloc_ctx;
};
@@ -687,7 +712,7 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
struct mx2_camera_dev *pcdev = ici->priv;
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
- writel((icd->user_width << 16) | icd->user_height,
+ writel((pcdev->s_width << 16) | pcdev->s_height,
pcdev->base_emma + PRP_SRC_FRAME_SIZE);
writel(prp->cfg.src_pixel,
pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
@@ -707,6 +732,74 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
writel(prp->cfg.irq_flags, pcdev->base_emma + PRP_INTR_CNTL);
}
+static void mx2_prp_resize_commit(struct mx2_camera_dev *pcdev)
+{
+ int dir;
+
+ for (dir = RESIZE_DIR_H; dir <= RESIZE_DIR_V; dir++) {
+ unsigned char *s = pcdev->resizing[dir].s;
+ int len = pcdev->resizing[dir].len;
+ unsigned int coeff[2] = {0, 0};
+ unsigned int valid = 0;
+ int i;
+
+ if (len == 0)
+ continue;
+
+ for (i = RESIZE_NUM_MAX - 1; i >= 0; i--) {
+ int j;
+
+ j = i > 9 ? 1 : 0;
+ coeff[j] = (coeff[j] << BC_COEF) |
+ (s[i] & (SZ_COEF - 1));
+
+ if (i == 5 || i == 15)
+ coeff[j] <<= 1;
+
+ valid = (valid << 1) | (s[i] >> BC_COEF);
+ }
+
+ valid |= PRP_RZ_VALID_TBL_LEN(len);
+
+ if (pcdev->resizing[dir].algo == RESIZE_ALGO_BILINEAR)
+ valid |= PRP_RZ_VALID_BILINEAR;
+
+ if (pcdev->emma_prp->cfg.channel == 1) {
+ if (dir == RESIZE_DIR_H) {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF1);
+ writel(coeff[1], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF2);
+ writel(valid, pcdev->base_emma +
+ PRP_CH1_RZ_HORI_VALID);
+ } else {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_VERT_COEF1);
+ writel(coeff[1], pcdev->base_emma +
+ PRP_CH1_RZ_VERT_COEF2);
+ writel(valid, pcdev->base_emma +
+ PRP_CH1_RZ_VERT_VALID);
+
Re: [PATCH v3 4/4] media i.MX27 camera: handle overflows properly.
Hi Guennadi,
On 20 February 2012 13:17, Guennadi Liakhovetski wrote:
>> @@ -1302,21 +1305,12 @@ static irqreturn_t mx27_camera_emma_irq(int
>> irq_emma, void *data)
>> __func__);
>>
>> 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);
>> - }
>> - if (((status & (3 << 5)) == (3 << 5)) ||
>> + buf = list_entry(pcdev->active_bufs.next,
>> + struct mx2_buffer, queue);
>> + mx27_camera_frame_done_emma(pcdev,
>> + buf->bufnum, true);
>> + status &= ~(1 << 7);
>> + } else if (((status & (3 << 5)) == (3 << 5)) ||
>
> This means, in case of an overflow you don't reset the channels any more?
> Is there a reason for that?
Apparently, while I added the "returning an error frame, and continue
with the next one" part I accidentally removed the "resetting the
buffer" part.
Let me send a v4 version of this patch. I hope to have it ready for tomorrow.
--
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] media: i.MX27 camera: Add resizing support.
len++;
>> +
>> + for (i = 1; i < nxt; i++) {
>> + if (len >= RESIZE_NUM_MAX)
>> + return -EINVAL;
>> + s[len] = 0;
>> + len++;
>> + }
>> + } while (carry != init_carry);
>> + }
>> + pcdev->resizing[dir].len = len;
>> + if (dir == RESIZE_DIR_H)
>> + mf_in->width = mf_in->width * den / num;
>> + else
>> + mf_in->height = mf_in->height * den / num;
>
> Aren't your calculations exact, so that here you can just use
> pix_out->width and pix_out->height?
Yes, they are. I can save these operations. Thank you.
>> + }
>> + return 0;
>> +}
>> +
>> static int mx2_camera_set_fmt(struct soc_camera_device *icd,
>> struct v4l2_format *f)
>> {
>> @@ -1070,6 +1278,9 @@ static int mx2_camera_set_fmt(struct soc_camera_device
>> *icd,
>> struct v4l2_mbus_framefmt mf;
>> int ret;
>>
>> + dev_dbg(icd->parent, "%s: requested params: width = %d, height = %d\n",
>> + __func__, pix->width, pix->height);
>> +
>> xlate = soc_camera_xlate_by_fourcc(icd, pix->pixelformat);
>> if (!xlate) {
>> dev_warn(icd->parent, "Format %x not found\n",
>> @@ -1087,6 +1298,18 @@ static int mx2_camera_set_fmt(struct
>> soc_camera_device *icd,
>> if (ret < 0 && ret != -ENOIOCTLCMD)
>> return ret;
>>
>> + /* Store width and height returned by the sensor for resizing */
>> + pcdev->s_width = mf.width;
>> + pcdev->s_height = mf.height;
>> + dev_dbg(icd->parent, "%s: sensor params: width = %d, height = %d\n",
>> + __func__, pcdev->s_width, pcdev->s_height);
>> +
>> + memset(pcdev->resizing, 0, sizeof(struct emma_prp_resize) << 1);
>
> I think, just sizeof(pcdev->resizing) will do the trick.
No problem.
>> + if (mf.width != pix->width || mf.height != pix->height) {
>> + if (mx2_emmaprp_resize(pcdev, &mf, pix) < 0)
>> + return -EINVAL;
>
> Hmmm... This looks like a regression, not an improvement - you return more
> errors now, than without this resizing support. Wouldn't it be possible to
> fall back to the current behaviour if prp resizing is impossible?
You are right. I did this for testing purposes and forgot to update it.
>> + }
>> +
>> if (mf.code != xlate->code)
>> return -EINVAL;
>>
>> @@ -1100,6 +1323,9 @@ static int mx2_camera_set_fmt(struct soc_camera_device
>> *icd,
>> pcdev->emma_prp = mx27_emma_prp_get_format(xlate->code,
>> xlate->host_fmt->fourcc);
>>
>> + dev_dbg(icd->parent, "%s: returned params: width = %d, height = %d\n",
>> + __func__, pix->width, pix->height);
>> +
>> return 0;
>> }
>>
>> @@ -1109,11 +1335,16 @@ static int mx2_camera_try_fmt(struct
>> soc_camera_device *icd,
>> struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
>> const struct soc_camera_format_xlate *xlate;
>> struct v4l2_pix_format *pix = &f->fmt.pix;
>> - struct v4l2_mbus_framefmt mf;
>> __u32 pixfmt = pix->pixelformat;
>> + struct v4l2_mbus_framefmt mf;
>
> This swap doesn't seem to be needed.
Fine, I'll fix it.
>> + struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
>> + struct mx2_camera_dev *pcdev = ici->priv;
>> unsigned int width_limit;
>> int ret;
>>
>> + dev_dbg(icd->parent, "%s: requested params: width = %d, height = %d\n",
>> + __func__, pix->width, pix->height);
>> +
>> xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
>> if (pixfmt && !xlate) {
>> dev_warn(icd->parent, "Format %x not found\n", pixfmt);
>> @@ -1163,6 +1394,19 @@ static int mx2_camera_try_fmt(struct
>> soc_camera_device *icd,
>> if (ret < 0)
>> return ret;
>>
>> + /* Store width and height returned by the sensor for resizing */
>> + pcdev->s_width = mf.width;
>> + pcdev->s_height = mf.height;
>
> You don't need these in .try_fmt().
Right.
>> + dev_dbg(icd->parent, "%s: sensor params: width = %d, height = %d\n",
>> + __func__, pcdev->s_width, pcdev->s_height);
>> +
>> + /* If the sensor does not support image size try PrP resizing */
>> + memset(pcdev->resizing, 0, sizeof(struct emma_prp_resize) << 1);
>
> Same for sizeof()
>
>> + if (mf.width != pix->width || mf.height != pix->height) {
>> + if (mx2_emmaprp_resize(pcdev, &mf, pix) < 0)
>> + return -EINVAL;
>
> .try_fmt() really shouldn't fail.
It's the same issue as with s_fmt, I will fix it.
I'll try to send a v2 tomorrow.
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] media: i.MX27 camera: Add resizing support.
On 21 February 2012 09:39, Guennadi Liakhovetski wrote: > Hi Javier > > One more thing occurred to me: I don't see anywhere in your patch checking > for supported pixel (fourcc) formats. I don't think the PRP can resize > arbitrary formats? Most likely these would be limited to some YUV, and, > possibly, some RGB formats? The PrP can resize every format which is supported as input by the eMMa. Currently, the driver supports 2 input formats: RGB565 and YUV422 (YUYV) (see mx27_emma_prp_table[]). Since the commit of resizing registers is done in the stream_start callback this makes sure that resizing won't be applied to unknown formats. 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] media: i.MX27 camera: Add resizing support.
On 21 February 2012 10:24, Guennadi Liakhovetski wrote: > On Tue, 21 Feb 2012, javier Martin wrote: > >> On 21 February 2012 09:39, Guennadi Liakhovetski >> wrote: >> > Hi Javier >> > >> > One more thing occurred to me: I don't see anywhere in your patch checking >> > for supported pixel (fourcc) formats. I don't think the PRP can resize >> > arbitrary formats? Most likely these would be limited to some YUV, and, >> > possibly, some RGB formats? >> >> The PrP can resize every format which is supported as input by the eMMa. >> >> Currently, the driver supports 2 input formats: RGB565 and YUV422 >> (YUYV) (see mx27_emma_prp_table[]). > > That's not how I understand it. The mx27_emma_prp_table[] array has 2 > entries: the first one is indeed configured for RGB565, and the second one > is converting input YUV422 to output YUV420. But the former one is not > really that specific format, rather it is a generic configuration used as > a pass-through mode for generic 16-bit formats. > > BTW, does that mean, that on i.MX27 the driver currently doesn't support > 8-bit formats like Bayer? According to the datasheet, the eMMa-PrP only accepts the following input formats when capturing data form the CSI: RGB 16 bpp RGB 32 bpp (unpacked RGB888) YUV 4:2:2 Pixel interleaved YUV 4:4:4 But the driver only supports: - RGB 16bpp which, as you say is used as pass-through mode for generic 16-bit formats. - YUV 422 which is converted to YUV420. I'm sorry, you are right. Since I only use the latter, I hadn't noticed that the resizing engine could in fact have problems with 16bit pass-through mode depending on what 16bit format is really being transfered. What I can do is restricting the use of resizing only to the YUV422 case so that someone who is using the old pass-through mode can add support for resizing later for this format. -- 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
[PATCH v4 4/4] media i.MX27 camera: handle overflows properly.
Signed-off-by: Javier Martin
---
Changes since v3:
- Reset the channels properly.
---
drivers/media/video/mx2_camera.c | 38 +++---
1 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 3880d24..cf958ec 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -1211,7 +1211,7 @@ static struct soc_camera_host_ops mx2_soc_camera_host_ops
= {
};
static void mx27_camera_frame_done_emma(struct mx2_camera_dev *pcdev,
- int bufnum)
+ int bufnum, bool err)
{
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
struct mx2_buffer *buf;
@@ -1258,7 +1258,10 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
list_del_init(&buf->queue);
do_gettimeofday(&vb->v4l2_buf.timestamp);
vb->v4l2_buf.sequence = pcdev->frame_count;
- vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
+ if (err)
+ vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
+ else
+ vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
}
pcdev->frame_count++;
@@ -1309,21 +1312,18 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
}
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);
+ u32 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);
- }
- if (((status & (3 << 5)) == (3 << 5)) ||
+
+ buf = list_entry(pcdev->active_bufs.next,
+ struct mx2_buffer, queue);
+ mx27_camera_frame_done_emma(pcdev,
+ buf->bufnum, true);
+
+ status &= ~(1 << 7);
+ } else if (((status & (3 << 5)) == (3 << 5)) ||
((status & (3 << 3)) == (3 << 3))) {
/*
* Both buffers have triggered, process the one we're expecting
@@ -1331,13 +1331,13 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
*/
buf = list_entry(pcdev->active_bufs.next,
struct mx2_buffer, queue);
- mx27_camera_frame_done_emma(pcdev, buf->bufnum);
+ mx27_camera_frame_done_emma(pcdev, buf->bufnum, false);
status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
+ } else if ((status & (1 << 6)) || (status & (1 << 4))) {
+ mx27_camera_frame_done_emma(pcdev, 0, false);
+ } else if ((status & (1 << 5)) || (status & (1 << 3))) {
+ mx27_camera_frame_done_emma(pcdev, 1, false);
}
- if ((status & (1 << 6)) || (status & (1 << 4)))
- mx27_camera_frame_done_emma(pcdev, 0);
- if ((status & (1 << 5)) || (status & (1 << 3)))
- mx27_camera_frame_done_emma(pcdev, 1);
irq_ok:
spin_unlock_irqrestore(&pcdev->lock, flags);
--
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
[PATCH v2 2/6] media: i.MX27 camera: Use list_first_entry() whenever possible.
Signed-off-by: Javier Martin
---
Changes since v1:
- Adapt to [PATCH v4 4/4] media i.MX27 camera: handle overflows properly.
---
drivers/media/video/mx2_camera.c | 26 --
1 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 0ade14e..7793264 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -458,7 +458,7 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
buf = NULL;
writel(0, pcdev->base_csi + fb_reg);
} else {
- buf = list_entry(pcdev->capture.next, struct mx2_buffer,
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
queue);
vb = &buf->vb;
list_del(&buf->queue);
@@ -718,8 +718,8 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
spin_lock_irqsave(&pcdev->lock, flags);
- buf = list_entry(pcdev->capture.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
+ queue);
buf->bufnum = 0;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
@@ -728,8 +728,8 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
mx27_update_emma_buf(pcdev, phys, buf->bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
- buf = list_entry(pcdev->capture.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
+ queue);
buf->bufnum = 1;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
@@ -1215,8 +1215,7 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
struct vb2_buffer *vb;
unsigned long phys;
- buf = list_entry(pcdev->active_bufs.next,
-struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->active_bufs, struct mx2_buffer, queue);
BUG_ON(buf->bufnum != bufnum);
@@ -1270,8 +1269,8 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
return;
}
- buf = list_entry(pcdev->discard.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->discard, struct mx2_buffer,
+ queue);
buf->bufnum = bufnum;
list_move_tail(pcdev->discard.next, &pcdev->active_bufs);
@@ -1279,8 +1278,7 @@ static void mx27_camera_frame_done_emma(struct
mx2_camera_dev *pcdev,
return;
}
- buf = list_entry(pcdev->capture.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->capture, struct mx2_buffer, queue);
buf->bufnum = bufnum;
@@ -1314,7 +1312,7 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
pcdev->base_emma + PRP_CNTL);
writel(cntl, pcdev->base_emma + PRP_CNTL);
- buf = list_entry(pcdev->active_bufs.next,
+ buf = list_first_entry(pcdev->active_bufs.next,
struct mx2_buffer, queue);
mx27_camera_frame_done_emma(pcdev,
buf->bufnum, true);
@@ -1326,8 +1324,8 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma,
void *data)
* Both buffers have triggered, process the one we're expecting
* to first
*/
- buf = list_entry(pcdev->active_bufs.next,
- struct mx2_buffer, queue);
+ buf = list_first_entry(&pcdev->active_bufs, struct mx2_buffer,
+ queue);
mx27_camera_frame_done_emma(pcdev, buf->bufnum, false);
status &= ~(1 << (6 - buf->bufnum)); /* mark processed */
} else if ((status & (1 << 6)) || (status & (1 << 4))) {
--
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
[PATCH v2 6/6] media: i.MX27 camera: more efficient discard buffer handling.
Some elements of 'mx2_buffer' are grouped together in another
auxiliary structure. This way we don't need to have unused
'vb2_buffer' structures for both discard buffers.
Signed-off-by: Javier Martin
---
Changes since v1:
- Adapt to [PATCH v4 4/4] media i.MX27 camera: handle overflows properly.
---
drivers/media/video/mx2_camera.c | 77 ++
1 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 96a10af..fcb6b3f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -230,15 +230,18 @@ enum mx2_buffer_state {
MX2_STATE_DONE,
};
+struct mx2_buf_internal {
+ struct list_headqueue;
+ int bufnum;
+ booldiscard;
+};
+
/* buffer for one video frame */
struct mx2_buffer {
/* common v4l buffer stuff -- must be first */
struct vb2_buffer vb;
- struct list_headqueue;
enum mx2_buffer_state state;
-
- int bufnum;
- booldiscard;
+ struct mx2_buf_internal internal;
};
struct mx2_camera_dev {
@@ -270,7 +273,7 @@ struct mx2_camera_dev {
u32 csicr1;
- struct mx2_buffer buf_discard[2];
+ struct mx2_buf_internal buf_discard[2];
void*discard_buffer;
dma_addr_t discard_buffer_dma;
size_t discard_size;
@@ -279,6 +282,11 @@ struct mx2_camera_dev {
struct vb2_alloc_ctx*alloc_ctx;
};
+static struct mx2_buffer *mx2_ibuf_to_buf(struct mx2_buf_internal *int_buf)
+{
+ return container_of(int_buf, struct mx2_buffer, internal);
+}
+
static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
/*
* This is a generic configuration which is valid for most
@@ -459,9 +467,9 @@ static void mx25_camera_frame_done(struct mx2_camera_dev
*pcdev, int fb,
writel(0, pcdev->base_csi + fb_reg);
} else {
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
+ internal.queue);
vb = &buf->vb;
- list_del(&buf->queue);
+ list_del(&buf->internal.queue);
buf->state = MX2_STATE_ACTIVE;
writel(vb2_dma_contig_plane_dma_addr(vb, 0),
pcdev->base_csi + fb_reg);
@@ -578,7 +586,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
spin_lock_irqsave(&pcdev->lock, flags);
buf->state = MX2_STATE_QUEUED;
- list_add_tail(&buf->queue, &pcdev->capture);
+ list_add_tail(&buf->internal.queue, &pcdev->capture);
if (cpu_is_mx25()) {
u32 csicr3, dma_inten = 0;
@@ -596,7 +604,7 @@ static void mx2_videobuf_queue(struct vb2_buffer *vb)
}
if (dma_inten) {
- list_del(&buf->queue);
+ list_del(&buf->internal.queue);
buf->state = MX2_STATE_ACTIVE;
csicr3 = readl(pcdev->base_csi + CSICR3);
@@ -719,23 +727,23 @@ static int mx2_start_streaming(struct vb2_queue *q,
unsigned int count)
spin_lock_irqsave(&pcdev->lock, flags);
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
- buf->bufnum = 0;
+ internal.queue);
+ buf->internal.bufnum = 0;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
phys = vb2_dma_contig_plane_dma_addr(vb, 0);
- mx27_update_emma_buf(pcdev, phys, buf->bufnum);
+ mx27_update_emma_buf(pcdev, phys, buf->internal.bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
buf = list_first_entry(&pcdev->capture, struct mx2_buffer,
- queue);
- buf->bufnum = 1;
+ internal.queue);
+ buf->internal.bufnum = 1;
vb = &buf->vb;
buf->state = MX2_STATE_ACTIVE;
phys = vb2_dma_contig_plane_dma_addr(vb, 0);
- mx27_update_emma_buf(pcdev, phys, buf->bufnum);
+ mx27_update_emma_buf(pcdev, phys, buf->internal.bufnum);
list_move_tail(pcdev->capture.next, &pcdev->active_bufs);
bytesperline = soc_mbus_bytes_per_line(icd->user_width,
@@ -1213,21 +1221,25 @@ static void mx27_came
Re: [PATCH v4 4/4] media i.MX27 camera: handle overflows properly.
Hi Guennadi, when you apply this patch don't forget to update two patches from the Clean up series which depend on this one: [PATCH v2 2/6] media: i.MX27 camera: Use list_first_entry() whenever possible. [PATCH v2 6/6] media: i.MX27 camera: more efficient discard buffer handling. The other ones apply fine as they are. 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] media: i.MX27 camera: Add resizing support.
>>> @@ -1087,6 +1298,18 @@ static int mx2_camera_set_fmt(struct >>> soc_camera_device *icd, >>> if (ret < 0 && ret != -ENOIOCTLCMD) >>> return ret; >>> >>> + /* Store width and height returned by the sensor for resizing */ >>> + pcdev->s_width = mf.width; >>> + pcdev->s_height = mf.height; >>> + dev_dbg(icd->parent, "%s: sensor params: width = %d, height = %d\n", >>> + __func__, pcdev->s_width, pcdev->s_height); >>> + >>> + memset(pcdev->resizing, 0, sizeof(struct emma_prp_resize) << 1); >> >> I think, just sizeof(pcdev->resizing) will do the trick. No, it doesn't work because pcdev->resizing is a pointer whose size is 4bytes. I will just left this unchanged with your permission. 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
[PATCH v2] media: i.MX27 camera: Add resizing support.
If the attached video sensor cannot provide the
requested image size, try to use resizing engine
included in the eMMa-PrP IP.
This patch supports both averaging and bilinear
algorithms.
Signed-off-by: Javier Martin
---
Changes since v1:
- Fix several cosmetic issues.
- Don't return -EINVAL if resizing is not possible.
- Only allow resizing for YUYV format at the moment.
- Simplify some lines of code.
---
drivers/media/video/mx2_camera.c | 256 +-
1 files changed, 252 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index fcb6b3f..453ad4c 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -204,10 +205,25 @@
#define PRP_INTR_LBOVF (1 << 7)
#define PRP_INTR_CH2OVF(1 << 8)
+/* Resizing registers */
+#define PRP_RZ_VALID_TBL_LEN(x)((x) << 24)
+#define PRP_RZ_VALID_BILINEAR (1 << 31)
+
#define mx27_camera_emma(pcdev)(cpu_is_mx27() && pcdev->use_emma)
#define MAX_VIDEO_MEM 16
+#define RESIZE_NUM_MIN 1
+#define RESIZE_NUM_MAX 20
+#define BC_COEF3
+#define SZ_COEF(1 << BC_COEF)
+
+#define RESIZE_DIR_H 0
+#define RESIZE_DIR_V 1
+
+#define RESIZE_ALGO_BILINEAR 0
+#define RESIZE_ALGO_AVERAGING 1
+
struct mx2_prp_cfg {
int channel;
u32 in_fmt;
@@ -217,6 +233,13 @@ struct mx2_prp_cfg {
u32 irq_flags;
};
+/* prp resizing parameters */
+struct emma_prp_resize {
+ int algo; /* type of algorithm used */
+ int len; /* number of coefficients */
+ unsigned char s[RESIZE_NUM_MAX]; /* table of coefficients */
+};
+
/* prp configuration for a client-host fmt pair */
struct mx2_fmt_cfg {
enum v4l2_mbus_pixelcodein_fmt;
@@ -278,6 +301,8 @@ struct mx2_camera_dev {
dma_addr_t discard_buffer_dma;
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
+ struct emma_prp_resize resizing[2];
+ unsigned ints_width, s_height;
u32 frame_count;
struct vb2_alloc_ctx*alloc_ctx;
};
@@ -687,7 +712,7 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
struct mx2_camera_dev *pcdev = ici->priv;
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
- writel((icd->user_width << 16) | icd->user_height,
+ writel((pcdev->s_width << 16) | pcdev->s_height,
pcdev->base_emma + PRP_SRC_FRAME_SIZE);
writel(prp->cfg.src_pixel,
pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
@@ -707,6 +732,74 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
writel(prp->cfg.irq_flags, pcdev->base_emma + PRP_INTR_CNTL);
}
+static void mx2_prp_resize_commit(struct mx2_camera_dev *pcdev)
+{
+ int dir;
+
+ for (dir = RESIZE_DIR_H; dir <= RESIZE_DIR_V; dir++) {
+ unsigned char *s = pcdev->resizing[dir].s;
+ int len = pcdev->resizing[dir].len;
+ unsigned int coeff[2] = {0, 0};
+ unsigned int valid = 0;
+ int i;
+
+ if (len == 0)
+ continue;
+
+ for (i = RESIZE_NUM_MAX - 1; i >= 0; i--) {
+ int j;
+
+ j = i > 9 ? 1 : 0;
+ coeff[j] = (coeff[j] << BC_COEF) |
+ (s[i] & (SZ_COEF - 1));
+
+ if (i == 5 || i == 15)
+ coeff[j] <<= 1;
+
+ valid = (valid << 1) | (s[i] >> BC_COEF);
+ }
+
+ valid |= PRP_RZ_VALID_TBL_LEN(len);
+
+ if (pcdev->resizing[dir].algo == RESIZE_ALGO_BILINEAR)
+ valid |= PRP_RZ_VALID_BILINEAR;
+
+ if (pcdev->emma_prp->cfg.channel == 1) {
+ if (dir == RESIZE_DIR_H) {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF1);
+ writel(coeff[1], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF2);
+ writel(valid, pcdev->base_emma +
+ PRP_CH1_RZ_HORI_VALID);
+ } else {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_VERT_COEF1);
+ writel(coeff[1], pcdev-
Re: [PATCH v2 2/6] media: i.MX27 camera: Use list_first_entry() whenever possible.
On 27 February 2012 09:49, Guennadi Liakhovetski wrote: > Hi Javier > > On Wed, 22 Feb 2012, Javier Martin wrote: > >> Signed-off-by: Javier Martin >> --- >> Changes since v1: >> - Adapt to [PATCH v4 4/4] media i.MX27 camera: handle overflows properly. >> >> --- >> drivers/media/video/mx2_camera.c | 26 -- >> 1 files changed, 12 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/media/video/mx2_camera.c >> b/drivers/media/video/mx2_camera.c >> index 0ade14e..7793264 100644 >> --- a/drivers/media/video/mx2_camera.c >> +++ b/drivers/media/video/mx2_camera.c > > [snip] > >> @@ -1314,7 +1312,7 @@ static irqreturn_t mx27_camera_emma_irq(int irq_emma, >> void *data) >> pcdev->base_emma + PRP_CNTL); >> writel(cntl, pcdev->base_emma + PRP_CNTL); >> >> - buf = list_entry(pcdev->active_bufs.next, >> + buf = list_first_entry(pcdev->active_bufs.next, > > This is the only hunk, that you've changed. I'll fix this to be > > + buf = list_first_entry(&pcdev->active_bufs, > >> struct mx2_buffer, queue); >> mx27_camera_frame_done_emma(pcdev, >> buf->bufnum, true); > Looks OK. 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
[PATCH v3] media: i.MX27 camera: Add resizing support.
If the attached video sensor cannot provide the
requested image size, try to use resizing engine
included in the eMMa-PrP IP.
This patch supports both averaging and bilinear
algorithms.
Signed-off-by: Javier Martin
---
Changes since v2:
- Don't modify controller private parameters in try_fmt.
---
drivers/media/video/mx2_camera.c | 260 +-
1 files changed, 256 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index fcb6b3f..8df624f 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -204,10 +205,25 @@
#define PRP_INTR_LBOVF (1 << 7)
#define PRP_INTR_CH2OVF(1 << 8)
+/* Resizing registers */
+#define PRP_RZ_VALID_TBL_LEN(x)((x) << 24)
+#define PRP_RZ_VALID_BILINEAR (1 << 31)
+
#define mx27_camera_emma(pcdev)(cpu_is_mx27() && pcdev->use_emma)
#define MAX_VIDEO_MEM 16
+#define RESIZE_NUM_MIN 1
+#define RESIZE_NUM_MAX 20
+#define BC_COEF3
+#define SZ_COEF(1 << BC_COEF)
+
+#define RESIZE_DIR_H 0
+#define RESIZE_DIR_V 1
+
+#define RESIZE_ALGO_BILINEAR 0
+#define RESIZE_ALGO_AVERAGING 1
+
struct mx2_prp_cfg {
int channel;
u32 in_fmt;
@@ -217,6 +233,13 @@ struct mx2_prp_cfg {
u32 irq_flags;
};
+/* prp resizing parameters */
+struct emma_prp_resize {
+ int algo; /* type of algorithm used */
+ int len; /* number of coefficients */
+ unsigned char s[RESIZE_NUM_MAX]; /* table of coefficients */
+};
+
/* prp configuration for a client-host fmt pair */
struct mx2_fmt_cfg {
enum v4l2_mbus_pixelcodein_fmt;
@@ -278,6 +301,8 @@ struct mx2_camera_dev {
dma_addr_t discard_buffer_dma;
size_t discard_size;
struct mx2_fmt_cfg *emma_prp;
+ struct emma_prp_resize resizing[2];
+ unsigned ints_width, s_height;
u32 frame_count;
struct vb2_alloc_ctx*alloc_ctx;
};
@@ -687,7 +712,7 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
struct mx2_camera_dev *pcdev = ici->priv;
struct mx2_fmt_cfg *prp = pcdev->emma_prp;
- writel((icd->user_width << 16) | icd->user_height,
+ writel((pcdev->s_width << 16) | pcdev->s_height,
pcdev->base_emma + PRP_SRC_FRAME_SIZE);
writel(prp->cfg.src_pixel,
pcdev->base_emma + PRP_SRC_PIXEL_FORMAT_CNTL);
@@ -707,6 +732,74 @@ static void mx27_camera_emma_buf_init(struct
soc_camera_device *icd,
writel(prp->cfg.irq_flags, pcdev->base_emma + PRP_INTR_CNTL);
}
+static void mx2_prp_resize_commit(struct mx2_camera_dev *pcdev)
+{
+ int dir;
+
+ for (dir = RESIZE_DIR_H; dir <= RESIZE_DIR_V; dir++) {
+ unsigned char *s = pcdev->resizing[dir].s;
+ int len = pcdev->resizing[dir].len;
+ unsigned int coeff[2] = {0, 0};
+ unsigned int valid = 0;
+ int i;
+
+ if (len == 0)
+ continue;
+
+ for (i = RESIZE_NUM_MAX - 1; i >= 0; i--) {
+ int j;
+
+ j = i > 9 ? 1 : 0;
+ coeff[j] = (coeff[j] << BC_COEF) |
+ (s[i] & (SZ_COEF - 1));
+
+ if (i == 5 || i == 15)
+ coeff[j] <<= 1;
+
+ valid = (valid << 1) | (s[i] >> BC_COEF);
+ }
+
+ valid |= PRP_RZ_VALID_TBL_LEN(len);
+
+ if (pcdev->resizing[dir].algo == RESIZE_ALGO_BILINEAR)
+ valid |= PRP_RZ_VALID_BILINEAR;
+
+ if (pcdev->emma_prp->cfg.channel == 1) {
+ if (dir == RESIZE_DIR_H) {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF1);
+ writel(coeff[1], pcdev->base_emma +
+ PRP_CH1_RZ_HORI_COEF2);
+ writel(valid, pcdev->base_emma +
+ PRP_CH1_RZ_HORI_VALID);
+ } else {
+ writel(coeff[0], pcdev->base_emma +
+ PRP_CH1_RZ_VERT_COEF1);
+ writel(coeff[1], pcdev->base_emma +
+ PRP_CH1_RZ_VERT_COEF2);
+ writel(valid, pcdev->base_emm
Re: [PATCH 1/2] media: tvp5150: Add cropping support.
Hi all, I've been using and testing these patches successfully for a month. Do you have any comments on them? Do you think they are ready to be merged? [PATCH 1/2] media: tvp5150: Add cropping support [PATCH 2/2] media: tvp5150: Add g_mbus_fmt_callback -- 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 1/2] media: tvp5150: Add cropping support.
On 2 March 2012 09:37, Marco Cavallini wrote: > javier Martin ha scritto, Il 02/03/2012 09:35: >> Hi all, >> I've been using and testing these patches successfully for a month. >> Do you have any comments on them? Do you think they are ready to be merged? >> >> [PATCH 1/2] media: tvp5150: Add cropping support >> [PATCH 2/2] media: tvp5150: Add g_mbus_fmt_callback >> >> > > Hi Javier, > I'm working on tvp5150 with a PXA270 just these days. > Could you provide a link to those patches please? Sure: http://www.spinics.net/lists/linux-media/msg43789.html http://www.spinics.net/lists/linux-media/msg43790.html -- 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
[Q] media: V4L2 compressed frames and s_fmt.
Hi, I'm developing a V4L2 mem2mem driver for the codadx6 IP video codec which is included in the i.MX27 chip. The capture interface of this driver can therefore return h.264 or mpeg4 video frames. Provided that the size of each frame varies and is unknown to the user, how is the driver supposed to react to a S_FMT when it comes to parameters such as the following? pix->width pix->height pix->bytesperline pix->sizeimage According to the documentation [1] I understand that the driver can just ignore 'bytesperline' and should return in 'sizeimage' the maximum buffer size to store a compressed frame. However, it does not mention anything special about width and height. Does it make sense setting width and height for h.264/mpeg4 formats? [1] http://v4l2spec.bytesex.org/spec/c2030.htm#V4L2-PIX-FORMAT -- 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: [Q] media: V4L2 compressed frames and s_fmt.
Hi Kamil, Sakari, thank you for your replies. On 15 March 2012 14:19, Kamil Debski wrote: > Hi Javier, Sakari, > >> From: Sakari Ailus [mailto:sakari.ai...@iki.fi] >> Sent: 15 March 2012 12:04 >> >> Hi Javier, >> >> (Cc Kamil.) >> >> On Wed, Mar 14, 2012 at 12:22:43PM +0100, javier Martin wrote: >> > Hi, >> > I'm developing a V4L2 mem2mem driver for the codadx6 IP video codec >> > which is included in the i.MX27 chip. >> > >> > The capture interface of this driver can therefore return h.264 or >> > mpeg4 video frames. >> > >> > Provided that the size of each frame varies and is unknown to the >> > user, how is the driver supposed to react to a S_FMT when it comes to >> > parameters such as the following? >> > >> > pix->width >> > pix->height >> > pix->bytesperline >> > pix->sizeimage >> > >> > According to the documentation [1] I understand that the driver can >> > just ignore 'bytesperline' and should return in 'sizeimage' the >> > maximum buffer size to store a compressed frame. However, it does not >> > mention anything special about width and height. Does it make sense >> > setting width and height for h.264/mpeg4 formats? >> > > Yes, in case of the compressed side (capture) the width, height and > bytesperline > is ignored. The MFC driver sets bytesperline to 0 and leaves width and height > intact > during S_FMT. I suggest you do the same or set all of them (width, height, > bytesperline) > to 0. I'm not sure about that, according to the code in here [1] it ignores width and height, as you stated, but it fills bytesperline with the value in imagesize. This applies to TRY_FMT and S_FMT. On the other hand, in G_FMT [2], it sets width and height to 0, but bytesperline and sizeimage are set to ctx->enc_dst_buf_size, which I deduce it's the encoder buffer size. If this is the agreed way of doing things I can just implement this behavior in my driver as well. Regards. [1] http://lxr.linux.no/#linux+v3.2.11/drivers/media/video/s5p-mfc/s5p_mfc_enc.c#L880 [2] http://lxr.linux.no/#linux+v3.2.11/drivers/media/video/s5p-mfc/s5p_mfc_enc.c#L844 -- 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
[Q] ov7670: green line in VGA resolution
Hi, I am currently testing an ov7670 sensor against my mx2_camera.c soc-camera driver. Everything works as expected with the exception of a green vertical line of about 6-7 pixels width that appears on the left side of the image. I suspect the problem is related to the fact that this sensor has an array of 656 x 488 pixels but only 640 x 480 are active. The datasheet available from Omnivision (Version 1.4, August 21, 2006) is not clear about how to configure the sensor not to show non active pixels but I could find the following patch which addresses a similar problem for QVGA: http://kernel.ubuntu.com/git?p=bradf/backup.ubuntu-maverick/.git;a=commitdiff;h=e4182762eaf3c80b2f5c8d1d373409d7c2843579;hp=e770cc1e35a3f11cffd1f38f52060e3e38b4fbf7 But I don't know how these values can be extrapolated to the VGA case. Has anybody found the same issue? 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: [Q] ov7670: green line in VGA resolution
Hi Jonathan, thank you for your attention. On 19 March 2012 18:44, Jonathan Corbet wrote: > On Mon, 19 Mar 2012 17:27:06 +0100 > javier Martin wrote: > >> I suspect the problem is related to the fact that this sensor has an >> array of 656 x 488 pixels but only 640 x 480 are active. The datasheet >> available from Omnivision (Version 1.4, August 21, 2006) is not clear >> about how to configure the sensor not to show non active pixels but I >> could find the following patch which addresses a similar problem for >> QVGA: > > Interesting...nobody ever sent that patch anywhere where I've seen it. > > Anyway, the ov7670 datasheet is not clear on much of anything, and the > things it *is* clear on are likely to be wrong. > > The comment in the patch makes it clear how this was worked out, anyway: > "empirically determined." Unless you can get through to the one person at > OmniVision who knows how this sensor actually works, the best that can be > done is to mess with the values for the window. That's often done at both > the sensor and the controller level - if you look at the Marvell > controller, you'll see window tweaking there too. So, what I understand is that you see the same green line and, due to the lack of documentation for the ov7670, you solve it adjusting de video window in the Marvell controller driver. Could you confirm this? 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
[PATCH] media: i.MX2: eMMa-PrP: Allow userptr IO mode.
Userptr can be very useful if this device is requested to use video buffers allocated by another processing device. So that buffers don't need to be copied. Signed-off-by: Javier Martin --- drivers/media/video/mx2_emmaprp.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/mx2_emmaprp.c b/drivers/media/video/mx2_emmaprp.c index ba89a74..55ac173 100644 --- a/drivers/media/video/mx2_emmaprp.c +++ b/drivers/media/video/mx2_emmaprp.c @@ -755,7 +755,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(src_vq, 0, sizeof(*src_vq)); src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT; - src_vq->io_modes = VB2_MMAP; + src_vq->io_modes = VB2_MMAP | VB2_USERPTR; src_vq->drv_priv = ctx; src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); src_vq->ops = &emmaprp_qops; @@ -767,7 +767,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, memset(dst_vq, 0, sizeof(*dst_vq)); dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - dst_vq->io_modes = VB2_MMAP; + dst_vq->io_modes = VB2_MMAP | VB2_USERPTR; dst_vq->drv_priv = ctx; dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); dst_vq->ops = &emmaprp_qops; -- 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
[Q] What sensor does s5K6aa driver support?
Hi, the following driver claims to support S5K6AAFX Samsung sensor: http://lxr.linux.no/#linux+v3.3/drivers/media/video/s5k6aa.c However, we've been informed that this reference is marked as EOL. Has anyone tested this driver with anothe sensor compatible with S5K6AAFX that can be used as a replacement? 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
[PATCH 0/3] media: tvp5150: Fix mbus format to UYUV instead of YUYV.
[PATCH 1/3] media: tvp5150: Fix mbus format. [PATCH 2/3] media: mx2_camera: Fix mbus format handling. [PATCH 3/3] i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16. -- 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 1/3] media: tvp5150: Fix mbus format.
According to p.14 fig 3-3 of the datasheet (SLES098A) this decoder transmits data in UYVY format. Signed-off-by: Javier Martin --- drivers/media/video/tvp5150.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c index e292c46..30c88e0 100644 --- a/drivers/media/video/tvp5150.c +++ b/drivers/media/video/tvp5150.c @@ -821,7 +821,7 @@ static int tvp5150_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned index, if (index) return -EINVAL; - *code = V4L2_MBUS_FMT_YUYV8_2X8; + *code = V4L2_MBUS_FMT_UYVY8_2X8; return 0; } @@ -845,7 +845,7 @@ static int tvp5150_mbus_fmt(struct v4l2_subdev *sd, f->width = decoder->rect.width; f->height = decoder->rect.height; - f->code = V4L2_MBUS_FMT_YUYV8_2X8; + f->code = V4L2_MBUS_FMT_UYVY8_2X8; f->field = V4L2_FIELD_SEQ_TB; f->colorspace = V4L2_COLORSPACE_SMPTE170M; -- 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
[PATCH 2/3] media: mx2_camera: Fix mbus format handling.
Remove MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags
so that the driver can negotiate with the attached sensor
whether the mbus format needs convertion from UYUV to YUYV
or not.
Signed-off-by: Javier Martin
---
arch/arm/plat-mxc/include/mach/mx2_cam.h |2 -
drivers/media/video/mx2_camera.c | 52 +++---
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/mx2_cam.h
b/arch/arm/plat-mxc/include/mach/mx2_cam.h
index 3c080a3..7ded6f1 100644
--- a/arch/arm/plat-mxc/include/mach/mx2_cam.h
+++ b/arch/arm/plat-mxc/include/mach/mx2_cam.h
@@ -23,7 +23,6 @@
#ifndef __MACH_MX2_CAM_H_
#define __MACH_MX2_CAM_H_
-#define MX2_CAMERA_SWAP16 (1 << 0)
#define MX2_CAMERA_EXT_VSYNC (1 << 1)
#define MX2_CAMERA_CCIR(1 << 2)
#define MX2_CAMERA_CCIR_INTERLACE (1 << 3)
@@ -31,7 +30,6 @@
#define MX2_CAMERA_GATED_CLOCK (1 << 5)
#define MX2_CAMERA_INV_DATA(1 << 6)
#define MX2_CAMERA_PCLK_SAMPLE_RISING (1 << 7)
-#define MX2_CAMERA_PACK_DIR_MSB(1 << 8)
/**
* struct mx2_camera_platform_data - optional platform data for mx2_camera
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 8df624f..9274a53 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -348,6 +348,19 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
PRP_INTR_CH2OVF,
}
},
+ {
+ .in_fmt = V4L2_MBUS_FMT_UYVY8_2X8,
+ .out_fmt= V4L2_PIX_FMT_YUV420,
+ .cfg= {
+ .channel= 2,
+ .in_fmt = PRP_CNTL_DATA_IN_YUV422,
+ .out_fmt= PRP_CNTL_CH2_OUT_YUV420,
+ .src_pixel = 0x22000888, /* YUV422 (YUYV) */
+ .irq_flags = PRP_INTR_RDERR | PRP_INTR_CH2WERR |
+ PRP_INTR_CH2FC | PRP_INTR_LBOVF |
+ PRP_INTR_CH2OVF,
+ }
+ },
};
static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
@@ -990,6 +1003,7 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
struct mx2_camera_dev *pcdev = ici->priv;
struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,};
+ const struct soc_camera_format_xlate *xlate;
unsigned long common_flags;
int ret;
int bytesperline;
@@ -1034,14 +1048,31 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
return ret;
}
+ xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
+ if (!xlate) {
+ dev_warn(icd->parent, "Format %x not found\n", pixfmt);
+ return -EINVAL;
+ }
+
+ if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) {
+ csicr1 |= CSICR1_PACK_DIR;
+ csicr1 &= ~CSICR1_SWAP16_EN;
+ dev_dbg(icd->parent, "already yuyv format, don't convert\n");
+ } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) {
+ csicr1 &= ~CSICR1_PACK_DIR;
+ csicr1 |= CSICR1_SWAP16_EN;
+ dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n");
+ } else {
+ dev_warn(icd->parent, "mbus format not supported\n");
+ return -EINVAL;
+ }
+
if (common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
csicr1 |= CSICR1_REDGE;
if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
csicr1 |= CSICR1_SOF_POL;
if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
csicr1 |= CSICR1_HSYNC_POL;
- if (pcdev->platform_flags & MX2_CAMERA_SWAP16)
- csicr1 |= CSICR1_SWAP16_EN;
if (pcdev->platform_flags & MX2_CAMERA_EXT_VSYNC)
csicr1 |= CSICR1_EXT_VSYNC;
if (pcdev->platform_flags & MX2_CAMERA_CCIR)
@@ -1052,8 +1083,6 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
csicr1 |= CSICR1_GCLK_MODE;
if (pcdev->platform_flags & MX2_CAMERA_INV_DATA)
csicr1 |= CSICR1_INV_DATA;
- if (pcdev->platform_flags & MX2_CAMERA_PACK_DIR_MSB)
- csicr1 |= CSICR1_PACK_DIR;
pcdev->csicr1 = csicr1;
@@ -1128,7 +1157,8 @@ static int mx2_camera_get_formats(struct
soc_camera_device *icd,
return 0;
}
- if (code == V4L2_MBUS_FMT_YUYV8_2X8) {
+ if (code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+ code == V4L2_MBUS_FMT_UYVY8_2X8) {
formats++;
[PATCH 3/3] i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16.
Signed-off-by: Javier Martin
---
arch/arm/mach-imx/mach-imx27_visstrim_m10.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index 3128cfe..4db00c6 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -164,7 +164,7 @@ static struct platform_device visstrim_tvp5150 = {
static struct mx2_camera_platform_data visstrim_camera = {
- .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
MX2_CAMERA_SWAP16 | MX2_CAMERA_PCLK_SAMPLE_RISING,
+ .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
MX2_CAMERA_PCLK_SAMPLE_RISING,
.clk = 10,
};
--
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
Re: [PATCH 3/3] i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16.
On 26 March 2012 14:10, Baruch Siach wrote:
> Hi Javier,
>
> On Mon, Mar 26, 2012 at 01:20:04PM +0200, Javier Martin wrote:
>>
>> Signed-off-by: Javier Martin
>> ---
>> arch/arm/mach-imx/mach-imx27_visstrim_m10.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
>> b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
>> index 3128cfe..4db00c6 100644
>> --- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
>> +++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
>> @@ -164,7 +164,7 @@ static struct platform_device visstrim_tvp5150 = {
>>
>>
>> static struct mx2_camera_platform_data visstrim_camera = {
>> - .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
>> MX2_CAMERA_SWAP16 | MX2_CAMERA_PCLK_SAMPLE_RISING,
>> + .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
>> MX2_CAMERA_PCLK_SAMPLE_RISING,
>> .clk = 10,
>> };
>
> The order of the last two patches in this series should be switched to
> preserve bisectability.
>
> baruch
You are right.
Thanks.
--
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
[PATCH v2 0/3] media: tvp5150: Fix mbus format to UYUV instead of YUYV.
Changes since v2: - Swap order of patches 3 and 4 to make the series bisectable. [PATCH v2 1/3] media: tvp5150: Fix mbus format. [PATCH v2 2/3] i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16. [PATCH v2 3/3] media: mx2_camera: Fix mbus format handling. -- 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 v2 1/3] media: tvp5150: Fix mbus format.
According to p.14 fig 3-3 of the datasheet (SLES098A) this decoder transmits data in UYVY format. Signed-off-by: Javier Martin --- drivers/media/video/tvp5150.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/video/tvp5150.c b/drivers/media/video/tvp5150.c index e292c46..30c88e0 100644 --- a/drivers/media/video/tvp5150.c +++ b/drivers/media/video/tvp5150.c @@ -821,7 +821,7 @@ static int tvp5150_enum_mbus_fmt(struct v4l2_subdev *sd, unsigned index, if (index) return -EINVAL; - *code = V4L2_MBUS_FMT_YUYV8_2X8; + *code = V4L2_MBUS_FMT_UYVY8_2X8; return 0; } @@ -845,7 +845,7 @@ static int tvp5150_mbus_fmt(struct v4l2_subdev *sd, f->width = decoder->rect.width; f->height = decoder->rect.height; - f->code = V4L2_MBUS_FMT_YUYV8_2X8; + f->code = V4L2_MBUS_FMT_UYVY8_2X8; f->field = V4L2_FIELD_SEQ_TB; f->colorspace = V4L2_COLORSPACE_SMPTE170M; -- 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
[PATCH v2 2/3] i.MX27: visstrim_m10: Remove use of MX2_CAMERA_SWAP16.
Signed-off-by: Javier Martin
---
arch/arm/mach-imx/mach-imx27_visstrim_m10.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index 3128cfe..4db00c6 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -164,7 +164,7 @@ static struct platform_device visstrim_tvp5150 = {
static struct mx2_camera_platform_data visstrim_camera = {
- .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
MX2_CAMERA_SWAP16 | MX2_CAMERA_PCLK_SAMPLE_RISING,
+ .flags = MX2_CAMERA_CCIR | MX2_CAMERA_CCIR_INTERLACE |
MX2_CAMERA_PCLK_SAMPLE_RISING,
.clk = 10,
};
--
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
[PATCH v2 3/3] media: mx2_camera: Fix mbus format handling.
Remove MX2_CAMERA_SWAP16 and MX2_CAMERA_PACK_DIR_MSB flags
so that the driver can negotiate with the attached sensor
whether the mbus format needs convertion from UYUV to YUYV
or not.
Signed-off-by: Javier Martin
---
arch/arm/plat-mxc/include/mach/mx2_cam.h |2 -
drivers/media/video/mx2_camera.c | 52 +++---
2 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/arch/arm/plat-mxc/include/mach/mx2_cam.h
b/arch/arm/plat-mxc/include/mach/mx2_cam.h
index 3c080a3..7ded6f1 100644
--- a/arch/arm/plat-mxc/include/mach/mx2_cam.h
+++ b/arch/arm/plat-mxc/include/mach/mx2_cam.h
@@ -23,7 +23,6 @@
#ifndef __MACH_MX2_CAM_H_
#define __MACH_MX2_CAM_H_
-#define MX2_CAMERA_SWAP16 (1 << 0)
#define MX2_CAMERA_EXT_VSYNC (1 << 1)
#define MX2_CAMERA_CCIR(1 << 2)
#define MX2_CAMERA_CCIR_INTERLACE (1 << 3)
@@ -31,7 +30,6 @@
#define MX2_CAMERA_GATED_CLOCK (1 << 5)
#define MX2_CAMERA_INV_DATA(1 << 6)
#define MX2_CAMERA_PCLK_SAMPLE_RISING (1 << 7)
-#define MX2_CAMERA_PACK_DIR_MSB(1 << 8)
/**
* struct mx2_camera_platform_data - optional platform data for mx2_camera
diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
index 8df624f..9274a53 100644
--- a/drivers/media/video/mx2_camera.c
+++ b/drivers/media/video/mx2_camera.c
@@ -348,6 +348,19 @@ static struct mx2_fmt_cfg mx27_emma_prp_table[] = {
PRP_INTR_CH2OVF,
}
},
+ {
+ .in_fmt = V4L2_MBUS_FMT_UYVY8_2X8,
+ .out_fmt= V4L2_PIX_FMT_YUV420,
+ .cfg= {
+ .channel= 2,
+ .in_fmt = PRP_CNTL_DATA_IN_YUV422,
+ .out_fmt= PRP_CNTL_CH2_OUT_YUV420,
+ .src_pixel = 0x22000888, /* YUV422 (YUYV) */
+ .irq_flags = PRP_INTR_RDERR | PRP_INTR_CH2WERR |
+ PRP_INTR_CH2FC | PRP_INTR_LBOVF |
+ PRP_INTR_CH2OVF,
+ }
+ },
};
static struct mx2_fmt_cfg *mx27_emma_prp_get_format(
@@ -990,6 +1003,7 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
struct mx2_camera_dev *pcdev = ici->priv;
struct v4l2_mbus_config cfg = {.type = V4L2_MBUS_PARALLEL,};
+ const struct soc_camera_format_xlate *xlate;
unsigned long common_flags;
int ret;
int bytesperline;
@@ -1034,14 +1048,31 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
return ret;
}
+ xlate = soc_camera_xlate_by_fourcc(icd, pixfmt);
+ if (!xlate) {
+ dev_warn(icd->parent, "Format %x not found\n", pixfmt);
+ return -EINVAL;
+ }
+
+ if (xlate->code == V4L2_MBUS_FMT_YUYV8_2X8) {
+ csicr1 |= CSICR1_PACK_DIR;
+ csicr1 &= ~CSICR1_SWAP16_EN;
+ dev_dbg(icd->parent, "already yuyv format, don't convert\n");
+ } else if (xlate->code == V4L2_MBUS_FMT_UYVY8_2X8) {
+ csicr1 &= ~CSICR1_PACK_DIR;
+ csicr1 |= CSICR1_SWAP16_EN;
+ dev_dbg(icd->parent, "convert uyvy mbus format into yuyv\n");
+ } else {
+ dev_warn(icd->parent, "mbus format not supported\n");
+ return -EINVAL;
+ }
+
if (common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
csicr1 |= CSICR1_REDGE;
if (common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
csicr1 |= CSICR1_SOF_POL;
if (common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
csicr1 |= CSICR1_HSYNC_POL;
- if (pcdev->platform_flags & MX2_CAMERA_SWAP16)
- csicr1 |= CSICR1_SWAP16_EN;
if (pcdev->platform_flags & MX2_CAMERA_EXT_VSYNC)
csicr1 |= CSICR1_EXT_VSYNC;
if (pcdev->platform_flags & MX2_CAMERA_CCIR)
@@ -1052,8 +1083,6 @@ static int mx2_camera_set_bus_param(struct
soc_camera_device *icd,
csicr1 |= CSICR1_GCLK_MODE;
if (pcdev->platform_flags & MX2_CAMERA_INV_DATA)
csicr1 |= CSICR1_INV_DATA;
- if (pcdev->platform_flags & MX2_CAMERA_PACK_DIR_MSB)
- csicr1 |= CSICR1_PACK_DIR;
pcdev->csicr1 = csicr1;
@@ -1128,7 +1157,8 @@ static int mx2_camera_get_formats(struct
soc_camera_device *icd,
return 0;
}
- if (code == V4L2_MBUS_FMT_YUYV8_2X8) {
+ if (code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+ code == V4L2_MBUS_FMT_UYVY8_2X8) {
formats++;
Re: [PATCH 27/34] media: mx2_camera: use managed functions to clean up code
Hi, On 17 September 2012 11:11, Guennadi Liakhovetski wrote: > On Mon, 17 Sep 2012, Shawn Guo wrote: > >> Use managed functions to clean up the error handling code and function >> mx2_camera_remove(). Along with the change, a few variables get removed >> from struct mx2_camera_dev. >> >> Signed-off-by: Shawn Guo >> Cc: Guennadi Liakhovetski >> Cc: linux-media@vger.kernel.org > > (in case you want to push it via arm-soc) > > Acked-by: Guennadi Liakhovetski This patch breaks the driver: soc-camera-pdrv soc-camera-pdrv.0: Probing soc-camera-pdrv.0 Unable to handle kernel paging request at virtual address 656d6162 pgd = c0004000 [656d6162] *pgd= Internal error: Oops: 8005 [#1] PREEMPT ARM Modules linked in: CPU: 0Not tainted (3.6.0-rc5-01222-g3413fb1 #12) PC is at 0x656d6162 LR is at soc_camera_host_register+0x230/0x81c pc : [<656d6162>]lr : []psr: 4033 sp : c3025e48 ip : fp : r10: c03f236c r9 : r8 : 0001 r7 : r6 : c317d414 r5 : c30431a0 r4 : c317d600 r3 : 656d6163 r2 : c3025e18 r1 : 000c r0 : c317d600 Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA Thumb Segment kernel Control: 0005317f Table: a0004000 DAC: 0017 Process swapper (pid: 1, stack limit = 0xc3024270) Stack: (0xc3025e48 to 0xc3026000) 5e40: c3006460 c3192960 0043 c317d638 c005d624 5e60: 0043 c317d410 c317d478 c317d414 c02012f0 c317d410 0043 5e80: c31a9800 c31a9820 c022ca74 c300ab20 c306e4a8 c306e4a0 5ea0: c3c0 4013 80d0 c317d410 c317d410 c306e4a8 c306e4a0 0001 5ec0: c03f236c c02c8d50 c03535ee c317d410 c02c8a44 5ee0: c306e4a8 c306e4a8 c03fc478 c03fc478 0050 c03e0774 c03dc684 c0189d30 5f00: c306e4a8 c0188c3c c306e4a8 c306e4dc c03fc478 0050 c0188db8 5f20: c03fc478 c3025f30 c0188d58 c0187610 c300760c c3079ad0 c03fc478 c03fc478 5f40: c318e680 c03f6458 c0187d24 c03535ee c03535ee c3025f58 c03fc478 5f60: 0001 c04049c0 c01892c8 c03fc464 0001 c04049c0 5f80: 0050 c018a05c c03dc67c c03d4e84 c04049c0 c00087c8 0006 0006 5fa0: c03eeca0 c03dc67c 0007 c03dc67c 0007 c04049c0 c03c41a4 0050 5fc0: c03e0774 c03c42f4 0006 0006 c03c41a4 c03c4214 5fe0: c0014900 0013 c0014900 [] (soc_camera_host_register+0x230/0x81c) from [] (mx2_camera_probe+0x30c/0x3ac) [] (mx2_camera_probe+0x30c/0x3ac) from [] (platform_drv_probe+0x14/0x18) [] (platform_drv_probe+0x14/0x18) from [] (driver_probe_device+0xb0/0x1cc) [] (driver_probe_device+0xb0/0x1cc) from [] (__driver_attach+0x60/0x84) [] (__driver_attach+0x60/0x84) from [] (bus_for_each_dev+0x48/0x84) [] (bus_for_each_dev+0x48/0x84) from [] (bus_add_driver+0x9c/0x20c) [] (bus_add_driver+0x9c/0x20c) from [] (driver_register+0xa0/0x138) [] (driver_register+0xa0/0x138) from [] (platform_driver_probe+0x18/0x98) [] (platform_driver_probe+0x18/0x98) from [] (do_one_initcall+0x94/0x16c) [] (do_one_initcall+0x94/0x16c) from [] (kernel_init+0xe0/0x1ac) [] (kernel_init+0xe0/0x1ac) from [] (kernel_thread_exit+0x0/0x8) Code: bad PC value ---[ end trace 7f259a1ce2e10b1a ]--- Kernel panic - not syncing: Attempted to kill init! exitcode=0x000b -- 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 26/34] media: mx2_camera: remove dead code in mx2_camera_add_device
On 17 September 2012 10:18, Guennadi Liakhovetski wrote:
> Hi Shawn
>
> Thanks for the clean up. Would you like these patches to go via a single
> tree, presumably, arm-soc? In this case
>
> On Mon, 17 Sep 2012, Shawn Guo wrote:
>
>> This is a piece of code becoming dead since commit 2c9ba37 ([media]
>> V4L: mx2_camera: remove unsupported i.MX27 DMA mode, make EMMA
>> mandatory). It should have been removed together with the commit.
>> Remove it now.
>>
>> Signed-off-by: Shawn Guo
>> Cc: Guennadi Liakhovetski
>> Cc: linux-media@vger.kernel.org
>
Acked-by: Javier Martin
> Acked-by: Guennadi Liakhovetski
>
> Thanks
> Guennadi
>
>> ---
>> drivers/media/video/mx2_camera.c |4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/video/mx2_camera.c
>> b/drivers/media/video/mx2_camera.c
>> index 965427f..89c7e28 100644
>> --- a/drivers/media/video/mx2_camera.c
>> +++ b/drivers/media/video/mx2_camera.c
>> @@ -441,11 +441,9 @@ static int mx2_camera_add_device(struct
>> soc_camera_device *icd)
>>
>> csicr1 = CSICR1_MCLKEN;
>>
>> - if (cpu_is_mx27()) {
>> + if (cpu_is_mx27())
>> csicr1 |= CSICR1_PRP_IF_EN | CSICR1_FCC |
>> CSICR1_RXFF_LEVEL(0);
>> - } else if (cpu_is_mx27())
>> - csicr1 |= CSICR1_SOF_INTEN | CSICR1_RXFF_LEVEL(2);
>>
>> pcdev->csicr1 = csicr1;
>> writel(pcdev->csicr1, pcdev->base_csi + CSICR1);
>> --
>> 1.7.9.5
>>
>
> ---
> 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
--
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 28/34] media: mx2_camera: remove mach/hardware.h inclusion
p;& buf->state == MX2_STATE_ACTIVE) {
>> + if (is_imx25_camera(pcdev) && buf->state == MX2_STATE_ACTIVE) {
>> if (pcdev->fb1_active == buf) {
>> pcdev->csicr1 &= ~CSICR1_FB1_DMA_INTEN;
>> writel(0, pcdev->base_csi + CSIDMASA_FB1);
>> @@ -807,7 +839,7 @@ static int mx2_start_streaming(struct vb2_queue *q,
>> unsigned int count)
>> unsigned long phys;
>> int bytesperline;
>>
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> unsigned long flags;
>> if (count < 2)
>> return -EINVAL;
>> @@ -902,7 +934,7 @@ static int mx2_stop_streaming(struct vb2_queue *q)
>> void *b;
>> u32 cntl;
>>
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> spin_lock_irqsave(&pcdev->lock, flags);
>>
>> cntl = readl(pcdev->base_emma + PRP_CNTL);
>> @@ -1054,11 +1086,11 @@ static int mx2_camera_set_bus_param(struct
>> soc_camera_device *icd)
>> if (bytesperline < 0)
>> return bytesperline;
>>
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> ret = mx27_camera_emma_prp_reset(pcdev);
>> if (ret)
>> return ret;
>> - } else if (cpu_is_mx25()) {
>> + } else if (is_imx25_camera(pcdev)) {
>> writel((bytesperline * icd->user_height) >> 2,
>> pcdev->base_csi + CSIRXCNT);
>> writel((bytesperline << 16) | icd->user_height,
>> @@ -1351,7 +1383,7 @@ static int mx2_camera_try_fmt(struct soc_camera_device
>> *icd,
>> /* FIXME: implement MX27 limits */
>>
>> /* limit to MX25 hardware capabilities */
>> - if (cpu_is_mx25()) {
>> + if (is_imx25_camera(pcdev)) {
>> if (xlate->host_fmt->bits_per_sample <= 8)
>> width_limit = 0x * 4;
>> else
>> @@ -1685,6 +1717,20 @@ static int __devinit mx2_camera_probe(struct
>> platform_device *pdev)
>> goto exit;
>> }
>>
>> + pcdev->devtype = pdev->id_entry->driver_data;
>> + switch (pcdev->devtype) {
>> + case IMX25_CAMERA:
>> + pcdev->reg_csisr = CSISR_IMX25;
>> + pcdev->reg_csicr3 = CSICR3_IMX25;
>> + break;
>> + case IMX27_CAMERA:
>> + pcdev->reg_csisr = CSISR_IMX27;
>> + pcdev->reg_csicr3 = CSICR3_IMX27;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> pcdev->clk_csi = devm_clk_get(&pdev->dev, "ahb");
>> if (IS_ERR(pcdev->clk_csi)) {
>> dev_err(&pdev->dev, "Could not get csi clock\n");
>> @@ -1722,7 +1768,7 @@ static int __devinit mx2_camera_probe(struct
>> platform_device *pdev)
>> pcdev->dev = &pdev->dev;
>> platform_set_drvdata(pdev, pcdev);
>>
>> - if (cpu_is_mx25()) {
>> + if (is_imx25_camera(pcdev)) {
>> err = devm_request_irq(&pdev->dev, irq_csi, mx25_camera_irq, 0,
>> MX2_CAM_DRV_NAME, pcdev);
>> if (err) {
>> @@ -1731,7 +1777,7 @@ static int __devinit mx2_camera_probe(struct
>> platform_device *pdev)
>> }
>> }
>>
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> err = mx27_camera_emma_init(pdev);
>> if (err)
>> goto exit;
>> @@ -1742,7 +1788,7 @@ static int __devinit mx2_camera_probe(struct
>> platform_device *pdev)
>> pcdev->soc_host.priv= pcdev;
>> pcdev->soc_host.v4l2_dev.dev= &pdev->dev;
>> pcdev->soc_host.nr = pdev->id;
>> - if (cpu_is_mx25())
>> + if (is_imx25_camera(pcdev))
>> pcdev->soc_host.capabilities = SOCAM_HOST_CAP_STRIDE;
>>
>> pcdev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
>> @@ -1762,7 +1808,7 @@ static int __devinit mx2_camera_probe(struct
>> platform_device *pdev)
>> exit_free_emma:
>> vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
>> eallocctx:
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> clk_disable_unprepare(pcdev->clk_emma_ipg);
>> clk_disable_unprepare(pcdev->clk_emma_ahb);
>> }
>> @@ -1780,7 +1826,7 @@ static int __devexit mx2_camera_remove(struct
>> platform_device *pdev)
>>
>> vb2_dma_contig_cleanup_ctx(pcdev->alloc_ctx);
>>
>> - if (cpu_is_mx27()) {
>> + if (is_imx27_camera(pcdev)) {
>> clk_disable_unprepare(pcdev->clk_emma_ipg);
>> clk_disable_unprepare(pcdev->clk_emma_ahb);
>> }
>> @@ -1794,6 +1840,7 @@ static struct platform_driver mx2_camera_driver = {
>> .driver = {
>> .name = MX2_CAM_DRV_NAME,
>> },
>> + .id_table = mx2_camera_devtype,
>> .remove = __devexit_p(mx2_camera_remove),
>> };
>>
>> --
>> 1.7.9.5
I can't test this patch because it depends heavily on the previous
one, which breaks the driver.
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 27/34] media: mx2_camera: use managed functions to clean up code
Hi Shawn, On 18 September 2012 09:43, Shawn Guo wrote: > On Mon, Sep 17, 2012 at 03:36:07PM +0200, javier Martin wrote: >> This patch breaks the driver: >> > Javier, > > Can you please apply the following change to see if it fixes the > problem? > > Shawn > > @@ -1783,6 +1783,8 @@ static int __devinit mx2_camera_probe(struct > platform_device *pdev) > goto exit; > } > > + platform_set_drvdata(pdev, NULL); > + > pcdev->soc_host.drv_name= MX2_CAM_DRV_NAME, > pcdev->soc_host.ops = &mx2_soc_camera_host_ops, > pcdev->soc_host.priv = pcdev; Yes. That fixes the problem. With this fix: Tested-by: Javier Martin 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 28/34] media: mx2_camera: remove mach/hardware.h inclusion
On 17 September 2012 15:59, Guennadi Liakhovetski wrote: > On Mon, 17 Sep 2012, javier Martin wrote: > >> Hi Shawn, >> >> On 17 September 2012 11:21, Guennadi Liakhovetski >> wrote: >> > On Mon, 17 Sep 2012, Shawn Guo wrote: >> > >> >> It changes the driver to use platform_device_id rather than cpu_is_xxx >> >> to determine the controller type, and updates the platform code >> >> accordingly. >> >> >> >> As the result, mach/hardware.h inclusion gets removed from the driver. >> >> >> >> Signed-off-by: Shawn Guo >> >> Cc: Guennadi Liakhovetski >> >> Cc: linux-media@vger.kernel.org >> > Tested-by: Javier Martin >> > Acked-by: Guennadi Liakhovetski >> >> i.MX25 support is broken and is scheduled for removal. > > It is not yet, I haven't pushed those your patches yet. > > Thanks > Guennadi > >> I think we should not keep on trying to maintain it. Couldn't we just >> drop it? It only makes maintenance tasks more difficult. >> >> > Thanks >> > Guennadi >> > >> >> --- >> >> arch/arm/mach-imx/clk-imx25.c |6 +- >> >> arch/arm/mach-imx/clk-imx27.c |6 +- >> >> arch/arm/mach-imx/devices/devices-common.h |1 + >> >> arch/arm/mach-imx/devices/platform-mx2-camera.c | 12 +-- >> >> drivers/media/video/mx2_camera.c| 95 >> >> +-- >> >> 5 files changed, 85 insertions(+), 35 deletions(-) >> >> >> >> diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c >> >> index 1aea073..71fe521 100644 >> >> --- a/arch/arm/mach-imx/clk-imx25.c >> >> +++ b/arch/arm/mach-imx/clk-imx25.c >> >> @@ -231,9 +231,9 @@ int __init mx25_clocks_init(void) >> >> clk_register_clkdev(clk[esdhc2_ipg_per], "per", >> >> "sdhci-esdhc-imx25.1"); >> >> clk_register_clkdev(clk[esdhc2_ipg], "ipg", "sdhci-esdhc-imx25.1"); >> >> clk_register_clkdev(clk[esdhc2_ahb], "ahb", "sdhci-esdhc-imx25.1"); >> >> - clk_register_clkdev(clk[csi_ipg_per], "per", "mx2-camera.0"); >> >> - clk_register_clkdev(clk[csi_ipg], "ipg", "mx2-camera.0"); >> >> - clk_register_clkdev(clk[csi_ahb], "ahb", "mx2-camera.0"); >> >> + clk_register_clkdev(clk[csi_ipg_per], "per", "imx25-camera.0"); >> >> + clk_register_clkdev(clk[csi_ipg], "ipg", "imx25-camera.0"); >> >> + clk_register_clkdev(clk[csi_ahb], "ahb", "imx25-camera.0"); >> >> clk_register_clkdev(clk[dummy], "audmux", NULL); >> >> clk_register_clkdev(clk[can1_ipg], NULL, "flexcan.0"); >> >> clk_register_clkdev(clk[can2_ipg], NULL, "flexcan.1"); >> >> diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c >> >> index 5ff5cf0..e26de52 100644 >> >> --- a/arch/arm/mach-imx/clk-imx27.c >> >> +++ b/arch/arm/mach-imx/clk-imx27.c >> >> @@ -224,7 +224,7 @@ int __init mx27_clocks_init(unsigned long fref) >> >> clk_register_clkdev(clk[per3_gate], "per", "imx-fb.0"); >> >> clk_register_clkdev(clk[lcdc_ipg_gate], "ipg", "imx-fb.0"); >> >> clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx-fb.0"); >> >> - clk_register_clkdev(clk[csi_ahb_gate], "ahb", "mx2-camera.0"); >> >> + clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0"); >> >> clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc"); >> >> clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc"); >> >> clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc"); >> >> @@ -251,8 +251,8 @@ int __init mx27_clocks_init(unsigned long fref) >> >> clk_register_clkdev(clk[i2c2_ipg_gate], NULL, "imx21-i2c.1"); >> >> clk_register_clkdev(clk[owire_ipg_gate], NULL, "mxc_w1.0"); >> >> clk_register_clkdev(clk[kpp_ipg_gate], NULL, "imx-keypad"); >> >> - clk_register_clkdev(clk[emma_ahb_gate], "emma-ahb", "mx2-camera.0");
Re: [GIT PULL v2] Initial i.MX5/CODA7 support for the CODA driver
Hi Richard, On 20 September 2012 05:32, Richard Zhao wrote: > why is it a request-pull? After 5 version of Philipp's patches we have agreed they are good enough to be merged; they don't break anything related to the old codadx6 while provide support for the new coda7: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/53627 The pull request is a way to tell Mauro this is ready to be merged in his linux-media tree and making things easier for him. 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: [GIT PULL v2] Initial i.MX5/CODA7 support for the CODA driver
On 20 September 2012 10:03, Richard Zhao wrote: > On Thu, Sep 20, 2012 at 09:10:46AM +0200, javier Martin wrote: >> Hi Richard, >> >> On 20 September 2012 05:32, Richard Zhao wrote: >> > why is it a request-pull? >> >> After 5 version of Philipp's patches we have agreed they are good >> enough to be merged; they don't break anything related to the old >> codadx6 while provide support for the new coda7: >> http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/53627 >> >> The pull request is a way to tell Mauro this is ready to be merged in >> his linux-media tree and making things easier for him. > I know the meaning. I just feel strange. Pull request is normally sent > by maintainer to up level maintainer who agreed to receive pull request. This was discussed some time ago here: http://lists.infradead.org/pipermail/linux-arm-kernel/2012-January/078862.html 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
[PATCH 0/5] media: ov7670: driver cleanup and support for ov7674.
The following series includes all the changes discussed in [1] that don't affect either bridge drivers that use ov7670 or soc-camera framework For this reason they are considered non controversial and sent separately. At least 1 more series will follow in order to implement all features described in [1]. [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html -- 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 1/5] media: ov7670: add support for ov7675.
ov7675 and ov7670 share the same registers but there is no way
to distinguish them at runtime. However, they require different
tweaks to achieve the desired resolution. For this reason this
patch adds a new ov7675 entry to the ov7670_id table.
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c | 164 ++--
1 file changed, 111 insertions(+), 53 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e7c82b2..0478a7b 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -183,6 +183,10 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define REG_HAECC7 0xaa/* Hist AEC/AGC control 7 */
#define REG_BD60MAX0xab/* 60hz banding step limit */
+enum ov7670_model {
+ MODEL_OV7670 = 0,
+ MODEL_OV7675,
+};
/*
* Information we maintain about a known sensor.
@@ -198,6 +202,7 @@ struct ov7670_info {
int clock_speed;/* External clock speed (MHz) */
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ enum ov7670_model model;
};
static inline struct ov7670_info *to_state(struct v4l2_subdev *sd)
@@ -652,7 +657,7 @@ static struct regval_list ov7670_qcif_regs[] = {
{ 0xff, 0xff },
};
-static struct ov7670_win_size {
+struct ov7670_win_size {
int width;
int height;
unsigned char com7_bit;
@@ -661,57 +666,105 @@ static struct ov7670_win_size {
int vstart; /* sense to humans, but evidently the sensor */
int vstop; /* will do the right thing... */
struct regval_list *regs; /* Regs to tweak */
-/* h/vref stuff */
-} ov7670_win_sizes[] = {
- /* VGA */
- {
- .width = VGA_WIDTH,
- .height = VGA_HEIGHT,
- .com7_bit = COM7_FMT_VGA,
- .hstart = 158, /* These values from */
- .hstop = 14, /* Omnivision */
- .vstart = 10,
- .vstop = 490,
- .regs = NULL,
- },
- /* CIF */
- {
- .width = CIF_WIDTH,
- .height = CIF_HEIGHT,
- .com7_bit = COM7_FMT_CIF,
- .hstart = 170, /* Empirically determined */
- .hstop = 90,
- .vstart = 14,
- .vstop = 494,
- .regs = NULL,
- },
- /* QVGA */
+};
+
+static struct ov7670_win_size ov7670_win_sizes[2][4] = {
+ /* ov7670 */
{
- .width = QVGA_WIDTH,
- .height = QVGA_HEIGHT,
- .com7_bit = COM7_FMT_QVGA,
- .hstart = 168, /* Empirically determined */
- .hstop = 24,
- .vstart = 12,
- .vstop = 492,
- .regs = NULL,
+ /* VGA */
+ {
+ .width = VGA_WIDTH,
+ .height = VGA_HEIGHT,
+ .com7_bit = COM7_FMT_VGA,
+ .hstart = 158, /* These values from */
+ .hstop = 14, /* Omnivision */
+ .vstart = 10,
+ .vstop = 490,
+ .regs = NULL,
+ },
+ /* CIF */
+ {
+ .width = CIF_WIDTH,
+ .height = CIF_HEIGHT,
+ .com7_bit = COM7_FMT_CIF,
+ .hstart = 170, /* Empirically determined */
+ .hstop = 90,
+ .vstart = 14,
+ .vstop = 494,
+ .regs = NULL,
+ },
+ /* QVGA */
+ {
+ .width = QVGA_WIDTH,
+ .height = QVGA_HEIGHT,
+ .com7_bit = COM7_FMT_QVGA,
+ .hstart = 168, /* Empirically determined */
+ .hstop = 24,
+ .vstart = 12,
+ .vstop = 492,
+ .regs = NULL,
+ },
+ /* QCIF */
+ {
+ .width = QCIF_WIDTH,
+ .height = QCIF_HEIGHT,
+ .com7_bit = COM7_FMT_VGA, /* see comment above */
+ .hstart = 456, /* Empirically determined */
+ .hstop
[PATCH 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
'min_height' and 'min_width' are variables that allow to specify the minimum
resolution that the sensor will achieve. This patch make v4l2 fmt callbacks
consider this parameters in order to return valid data to user space.
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 0478a7b..627fe5f 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -812,10 +812,11 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
struct ov7670_format_struct **ret_fmt,
struct ov7670_win_size **ret_wsize)
{
- int index;
+ int index, i;
struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
int n_win_sizes = ARRAY_SIZE(ov7670_win_sizes[info->model]);
+ int win_sizes_limit = n_win_sizes;
for (index = 0; index < N_OV7670_FMTS; index++)
if (ov7670_formats[index].mbus_code == fmt->code)
@@ -831,15 +832,30 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
* Fields: the OV devices claim to be progressive.
*/
fmt->field = V4L2_FIELD_NONE;
+
+ /*
+* Don't consider values that don't match min_height and min_width
+* constraints.
+*/
+ if (info->min_width || info->min_height)
+ for (i = 0; i < n_win_sizes; i++) {
+ wsize = ov7670_win_sizes[info->model] + i;
+
+ if (wsize->width < info->min_width ||
+ wsize->height < info->min_height) {
+ win_sizes_limit = i;
+ break;
+ }
+ }
/*
* Round requested image size down to the nearest
* we support, but not below the smallest.
*/
for (wsize = ov7670_win_sizes[info->model];
-wsize < ov7670_win_sizes[info->model] + n_win_sizes; wsize++)
+wsize < ov7670_win_sizes[info->model] + win_sizes_limit; wsize++)
if (fmt->width >= wsize->width && fmt->height >= wsize->height)
break;
- if (wsize >= ov7670_win_sizes[info->model] + n_win_sizes)
+ if (wsize >= ov7670_win_sizes[info->model] + win_sizes_limit)
wsize--; /* Take the smallest one */
if (ret_wsize != NULL)
*ret_wsize = wsize;
--
1.7.9.5
--
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/5] media: ov7670: add possibility to bypass pll for ov7675.
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c | 24 ++--
include/media/ov7670.h |1 +
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 175fbfc..54fb535 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -210,6 +210,7 @@ struct ov7670_info {
int clock_speed;/* External clock speed (MHz) */
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ bool pll_bypass;
enum ov7670_model model;
};
@@ -778,7 +779,12 @@ static void ov7670_get_framerate(struct v4l2_subdev *sd,
{
struct ov7670_info *info = to_state(sd);
u32 clkrc = info->clkrc;
- u32 pll_factor = PLL_FACTOR;
+ int pll_factor;
+
+ if (info->pll_bypass)
+ pll_factor = 1;
+ else
+ pll_factor = PLL_FACTOR;
clkrc++;
if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
@@ -794,7 +800,7 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd,
{
struct ov7670_info *info = to_state(sd);
u32 clkrc;
- u32 pll_factor = PLL_FACTOR;
+ int pll_factor;
int ret;
/*
@@ -804,6 +810,16 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd,
* pixclk = clock_speed / (clkrc + 1) * PLLfactor
*
*/
+ if (info->pll_bypass) {
+ pll_factor = 1;
+ ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS);
+ } else {
+ pll_factor = PLL_FACTOR;
+ ret = ov7670_write(sd, REG_DBLV, DBLV_X4);
+ }
+ if (ret < 0)
+ return ret;
+
if (tpf->numerator == 0 || tpf->denominator == 0) {
clkrc = 0;
} else {
@@ -831,6 +847,7 @@ static int ov7670_set_framerate(struct v4l2_subdev *sd,
ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
if (ret < 0)
return ret;
+
return ov7670_write(sd, REG_DBLV, DBLV_X4);
}
@@ -1689,6 +1706,9 @@ static int ov7670_probe(struct i2c_client *client,
if (config->clock_speed)
info->clock_speed = config->clock_speed;
+
+ if (config->pll_bypass && id->driver_data != MODEL_OV7670)
+ info->pll_bypass = true;
}
/* Make sure it's an ov7670 */
diff --git a/include/media/ov7670.h b/include/media/ov7670.h
index b133bc1..a68c8bb 100644
--- a/include/media/ov7670.h
+++ b/include/media/ov7670.h
@@ -15,6 +15,7 @@ struct ov7670_config {
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ bool pll_bypass;/* Choose whether to bypass the PLL */
};
#endif
--
1.7.9.5
--
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 3/5] media: ov7670: calculate framerate properly for ov7675.
According to the datasheet ov7675 uses a formula to achieve
the desired framerate that is different from the operations
done in the current code.
In fact, this formula should apply to ov7670 too. This would
mean that current code is wrong but, in order to preserve
compatibility, the new formula will be used for ov7675 only.
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c | 122 ++--
1 file changed, 105 insertions(+), 17 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 627fe5f..175fbfc 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -47,6 +47,8 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
*/
#define OV7670_I2C_ADDR 0x42
+#define PLL_FACTOR 4
+
/* Registers */
#define REG_GAIN 0x00/* Gain lower 8 bits (rest in vref) */
#define REG_BLUE 0x01/* blue gain */
@@ -164,6 +166,12 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define REG_GFIX 0x69/* Fix gain control */
+#define REG_DBLV 0x6b/* PLL control an debugging */
+#define DBLV_BYPASS0x00/* Bypass PLL */
+#define DBLV_X40x01/* clock x4 */
+#define DBLV_X60x10/* clock x6 */
+#define DBLV_X80x11/* clock x8 */
+
#define REG_REG76 0x76/* OV's name */
#define R76_BLKPCOR0x80/* Black pixel correction enable */
#define R76_WHTPCOR0x40/* White pixel correction enable */
@@ -765,6 +773,67 @@ static struct ov7670_win_size ov7670_win_sizes[2][4] = {
}
};
+static void ov7670_get_framerate(struct v4l2_subdev *sd,
+struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+ u32 clkrc = info->clkrc;
+ u32 pll_factor = PLL_FACTOR;
+
+ clkrc++;
+ if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
+ clkrc = (clkrc >> 1);
+
+ tpf->numerator = 1;
+ tpf->denominator = (5 * pll_factor * info->clock_speed) /
+ (4 * clkrc);
+}
+
+static int ov7670_set_framerate(struct v4l2_subdev *sd,
+struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+ u32 clkrc;
+ u32 pll_factor = PLL_FACTOR;
+ int ret;
+
+ /*
+* The formula is fps = 5/4*pixclk for YUV/RGB and
+* fps = 5/2*pixclk for RAW.
+*
+* pixclk = clock_speed / (clkrc + 1) * PLLfactor
+*
+*/
+ if (tpf->numerator == 0 || tpf->denominator == 0) {
+ clkrc = 0;
+ } else {
+ clkrc = (5 * pll_factor * info->clock_speed * tpf->numerator) /
+ (4 * tpf->denominator);
+ if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
+ clkrc = (clkrc << 1);
+ clkrc--;
+ }
+
+ /*
+* The datasheet claims that clkrc = 0 will divide the input clock by 1
+* but we've checked with an oscilloscope that it divides by 2 instead.
+* So, if clkrc = 0 just bypass the divider.
+*/
+ if (clkrc <= 0)
+ clkrc = CLK_EXT;
+ else if (clkrc > CLK_SCALE)
+ clkrc = CLK_SCALE;
+ info->clkrc = clkrc;
+
+ /* Recalculate frame rate */
+ ov7670_get_framerate(sd, tpf);
+
+ ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
+ if (ret < 0)
+ return ret;
+ return ov7670_write(sd, REG_DBLV, DBLV_X4);
+}
+
/*
* Store a set of start/stop values into the camera.
*/
@@ -939,10 +1008,15 @@ static int ov7670_g_parm(struct v4l2_subdev *sd, struct
v4l2_streamparm *parms)
memset(cp, 0, sizeof(struct v4l2_captureparm));
cp->capability = V4L2_CAP_TIMEPERFRAME;
- cp->timeperframe.numerator = 1;
- cp->timeperframe.denominator = info->clock_speed;
- if ((info->clkrc & CLK_EXT) == 0 && (info->clkrc & CLK_SCALE) > 1)
- cp->timeperframe.denominator /= (info->clkrc & CLK_SCALE);
+ if (info->model == MODEL_OV7670) {
+ /* legacy */
+ cp->timeperframe.numerator = 1;
+ cp->timeperframe.denominator = info->clock_speed;
+ if ((info->clkrc & CLK_EXT) == 0 && (info->clkrc & CLK_SCALE) >
1)
+ cp->timeperframe.denominator /= (info->clkrc &
CLK_SCALE);
+ } else {
+ ov7670_get_framerate(sd, &cp->timeperframe);
+ }
return 0;
}
@@ -958,18 +1032,23 @@ static int ov7670_s_parm(struct v4l2_subdev *sd, struct
v4l2_streamparm *parms)
if (cp->extendedmode != 0)
return -EINVAL;
- if (tpf->numerator == 0 || tpf->denominator == 0)
-
[PATCH 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c |8
include/media/ov7670.h |1 +
2 files changed, 9 insertions(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 54fb535..f7e4341 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -211,6 +211,7 @@ struct ov7670_info {
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
bool pll_bypass;
+ bool pclk_hb_disable;
enum ov7670_model model;
};
@@ -1709,6 +1710,9 @@ static int ov7670_probe(struct i2c_client *client,
if (config->pll_bypass && id->driver_data != MODEL_OV7670)
info->pll_bypass = true;
+
+ if (config->pclk_hb_disable)
+ info->pclk_hb_disable = true;
}
/* Make sure it's an ov7670 */
@@ -1735,6 +1739,10 @@ static int ov7670_probe(struct i2c_client *client,
tpf.denominator = 30;
ov7670_set_framerate(sd, &tpf);
}
+
+ if (info->pclk_hb_disable)
+ ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
+
return 0;
}
diff --git a/include/media/ov7670.h b/include/media/ov7670.h
index a68c8bb..1913d51 100644
--- a/include/media/ov7670.h
+++ b/include/media/ov7670.h
@@ -16,6 +16,7 @@ struct ov7670_config {
int clock_speed;/* External clock speed (MHz) */
bool use_smbus; /* Use smbus I/O instead of I2C */
bool pll_bypass;/* Choose whether to bypass the PLL */
+ bool pclk_hb_disable; /* Disable toggling pixclk during
horizontal blanking */
};
#endif
--
1.7.9.5
--
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 0/5] media: ov7670: driver cleanup and support for ov7674.
On 26 September 2012 11:47, Javier Martin wrote: > The following series includes all the changes discussed in [1] that > don't affect either bridge drivers that use ov7670 or soc-camera framework > For this reason they are considered non controversial and sent separately. > At least 1 more series will follow in order to implement all features > described in [1]. > > > > [1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg51778.html Support is for ov7675, not ov7674, sorry for the typo. -- 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 1/5] media: ov7670: add support for ov7675.
Hi Jonathan,
thank you for your time.
On 26 September 2012 18:40, Jonathan Corbet wrote:
> This is going to have to be quick, sorry...
>
> On Wed, 26 Sep 2012 11:47:53 +0200
> Javier Martin wrote:
>
>> +static struct ov7670_win_size ov7670_win_sizes[2][4] = {
>> + /* ov7670 */
>
> I must confess I don't like this; now we've got constants in an array that
> was automatically sized before and ov7670_win_sizes[info->model]
> everywhere. I'd suggest a separate array for each device and an
> ov7670_get_wsizes(model) function.
>
>> + /* CIF - WARNING: not tested for ov7675 */
>> + {
>
> ...and this is part of why I don't like it. My experience with this
> particular sensor says that, if it's not tested, it hasn't yet seen the
> magic-number tweaking required to actually make it work. Please don't
> claim to support formats that you don't know actually work, or I'll get
> stuck with the bug reports :)
Your concern makes a lot of sense. In fact, that was one of my doubts
whether to 'support' not tested formats or not.
Let me fix that in a new version.
--
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 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
On 26 September 2012 18:42, Jonathan Corbet wrote: > On Wed, 26 Sep 2012 11:47:54 +0200 > Javier Martin wrote: > >> 'min_height' and 'min_width' are variables that allow to specify the minimum >> resolution that the sensor will achieve. This patch make v4l2 fmt callbacks >> consider this parameters in order to return valid data to user space. >> > I'd have preferred to do this differently, perhaps backing toward larger > sizes if the minimum turns out to be violated. But so be it... > > Acked-by: Jonathan Corbet > > jon Thank you. I will have to modify this patch slightly when I fix the previous one though. -- 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 3/5] media: ov7670: calculate framerate properly for ov7675.
On 26 September 2012 18:50, Jonathan Corbet wrote: > On Wed, 26 Sep 2012 11:47:55 +0200 > Javier Martin wrote: > >> According to the datasheet ov7675 uses a formula to achieve >> the desired framerate that is different from the operations >> done in the current code. >> >> In fact, this formula should apply to ov7670 too. This would >> mean that current code is wrong but, in order to preserve >> compatibility, the new formula will be used for ov7675 only. > > At this point I couldn't tell you what the real situation is; it's been a > while and there's always a fair amount of black magic involved with > ov7670 configuration. I do appreciate attention to not breaking existing > users. Indeed, this sensor is the quirkier I've dealt with, with those magic values in non documented registers... >> +static void ov7670_get_framerate(struct v4l2_subdev *sd, >> + struct v4l2_fract *tpf) > > This bugs me, though. It's called ov7670_get_framerate() but it's getting > the rate for the ov7675 - confusing. Meanwhile the real ov7670 code > remains inline while ov7675 has its own function. Actually, I did this on purpose because I wanted to remark that this function should be valid for both models and because I expected that the old behaviour was removed sometime in the future. > Please make two functions, one of which is ov7675_get_framerate(), and call > the right one for the model. Same for the "set" functions, obviously. > Maybe what's really needed is a structure full of sensor-specific > operations? The get_wsizes() function could go there too. That would take > a lot of if statements out of the code. The idea of a structure of sensor-specific operations seems reasonable. Furthermore, I think we should encourage users to use the right formula in the future. For this reason we could define 4 functions ov7670_set_framerate_legacy() ov7670_get_framerate_legacy() ov7675_set_framerate() ov7675_get_framerate() >> + /* >> + * The datasheet claims that clkrc = 0 will divide the input clock by 1 >> + * but we've checked with an oscilloscope that it divides by 2 instead. >> + * So, if clkrc = 0 just bypass the divider. >> + */ > > Thanks for documenting this kind of thing. You are welcome. -- 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/5] media: ov7670: add possibility to bypass pll for ov7675.
On 26 September 2012 18:52, Jonathan Corbet wrote: > On Wed, 26 Sep 2012 11:47:56 +0200 > Javier Martin wrote: > >> >> Signed-off-by: Javier Martin > > This one needs a changelog - what does bypassing the PLL do and why might > you want to do it? Otherwise: As I stated in a previous patch, frame rate depends on the pixclk. Moreover: pixclk = xvclk / clkrc * PLLfactor Bypassing the PLL means that the PLL gets out of the way so, in practice, PLLfactor = 1 pixclk = xvclk / clkrc For a frame rate of 30 fps a pixclk of 24MHz is needed. Since we have a clean clock signal we want pixclk = xvclk. If one applies the formula in ov7670_set_framerate() with PLLfactor = 1 and clock_speed = 24 MHz the resulting clkrc = 1 which means that: pixclk = xvclk which is what we want > Acked-by: Jonathan Corbet Thank you. I will add a changelog when I send v2 of the series. 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 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
On 26 September 2012 18:52, Jonathan Corbet wrote: > On Wed, 26 Sep 2012 11:47:57 +0200 > Javier Martin wrote: > >> Signed-off-by: Javier Martin >> --- >> drivers/media/i2c/ov7670.c |8 >> include/media/ov7670.h |1 + >> 2 files changed, 9 insertions(+) > > Again, needs a changelog. Otherwise Our soc-camera host driver captures pixels during blanking periods if pixclk is enabled. In order to avoid capturing bogus data we need to disable pixclk during those blanking periods I'll add it to v2. > Acked-by: Jonathan Corbet > Thank you. -- 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
[PATCH v2 0/5] media: ov7670: driver cleanup and support for ov7674.
The following series includes all the changes discussed in [1] that don't affect either bridge drivers that use ov7670 or soc-camera framework For this reason they are considered non controversial and sent separately. At least 1 more series will follow in order to implement all features described in [1]. This v2 include changes requested by Jonathan Corbet. See each separate patch. [PATCH v2 1/5] media: ov7670: add support for ov7675. [PATCH v2 2/5] media: ov7670: make try_fmt() consistent with [PATCH v2 3/5] media: ov7670: calculate framerate properly for ov7675. [PATCH v2 4/5] media: ov7670: add possibility to bypass pll for ov7675. [PATCH v2 5/5] media: ov7670: Add possibility to disable pixclk during -- 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 v2 1/5] media: ov7670: add support for ov7675.
ov7675 and ov7670 share the same registers but there is no way
to distinguish them at runtime. However, they require different
tweaks to achieve the desired resolution. For this reason this
patch adds a new ov7675 entry to the ov7670_id table.
Signed-off-by: Javier Martin
---
Changes since v1:
- Use separate arrays for supported resolutions.
- Don't support not tested resolutions in ov7675.
---
drivers/media/i2c/ov7670.c | 101 +++-
1 file changed, 72 insertions(+), 29 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e7c82b2..17eb5d7 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -183,6 +183,27 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define REG_HAECC7 0xaa/* Hist AEC/AGC control 7 */
#define REG_BD60MAX0xab/* 60hz banding step limit */
+enum ov7670_model {
+ MODEL_OV7670 = 0,
+ MODEL_OV7675,
+};
+
+struct ov7670_win_size {
+ int width;
+ int height;
+ unsigned char com7_bit;
+ int hstart; /* Start/stop values for the camera. Note */
+ int hstop; /* that they do not always make complete */
+ int vstart; /* sense to humans, but evidently the sensor */
+ int vstop; /* will do the right thing... */
+ struct regval_list *regs; /* Regs to tweak */
+};
+
+struct ov7670_devtype {
+ /* formats supported for each model */
+ struct ov7670_win_size *win_sizes;
+ unsigned int n_win_sizes;
+};
/*
* Information we maintain about a known sensor.
@@ -198,6 +219,7 @@ struct ov7670_info {
int clock_speed;/* External clock speed (MHz) */
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ const struct ov7670_devtype *devtype; /* Device specifics */
};
static inline struct ov7670_info *to_state(struct v4l2_subdev *sd)
@@ -652,65 +674,70 @@ static struct regval_list ov7670_qcif_regs[] = {
{ 0xff, 0xff },
};
-static struct ov7670_win_size {
- int width;
- int height;
- unsigned char com7_bit;
- int hstart; /* Start/stop values for the camera. Note */
- int hstop; /* that they do not always make complete */
- int vstart; /* sense to humans, but evidently the sensor */
- int vstop; /* will do the right thing... */
- struct regval_list *regs; /* Regs to tweak */
-/* h/vref stuff */
-} ov7670_win_sizes[] = {
+static struct ov7670_win_size ov7670_win_sizes[] = {
/* VGA */
{
.width = VGA_WIDTH,
.height = VGA_HEIGHT,
.com7_bit = COM7_FMT_VGA,
- .hstart = 158, /* These values from */
- .hstop = 14, /* Omnivision */
+ .hstart = 158, /* These values from */
+ .hstop = 14, /* Omnivision */
.vstart = 10,
.vstop = 490,
- .regs = NULL,
+ .regs = NULL,
},
/* CIF */
{
.width = CIF_WIDTH,
.height = CIF_HEIGHT,
.com7_bit = COM7_FMT_CIF,
- .hstart = 170, /* Empirically determined */
+ .hstart = 170, /* Empirically determined */
.hstop = 90,
.vstart = 14,
.vstop = 494,
- .regs = NULL,
+ .regs = NULL,
},
/* QVGA */
{
.width = QVGA_WIDTH,
.height = QVGA_HEIGHT,
.com7_bit = COM7_FMT_QVGA,
- .hstart = 168, /* Empirically determined */
+ .hstart = 168, /* Empirically determined */
.hstop = 24,
.vstart = 12,
.vstop = 492,
- .regs = NULL,
+ .regs = NULL,
},
/* QCIF */
{
.width = QCIF_WIDTH,
.height = QCIF_HEIGHT,
.com7_bit = COM7_FMT_VGA, /* see comment above */
- .hstart = 456, /* Empirically determined */
+ .hstart = 456, /* Empirically determined */
.hstop = 24,
.vstart = 14,
.vstop = 494,
- .regs = ov7670_qcif_regs,
- },
+ .regs = ov7670_qcif_regs,
+ }
};
-#define N_WIN_SIZES (ARRAY_SIZE(ov7670_win_si
[PATCH v2 3/5] media: ov7670: calculate framerate properly for ov7675.
According to the datasheet ov7675 uses a formula to achieve
the desired framerate that is different from the operations
done in the current code.
In fact, this formula should apply to ov7670 too. This would
mean that current code is wrong but, in order to preserve
compatibility, the new formula will be used for ov7675 only.
Signed-off-by: Javier Martin
---
Changes since v1:
- Create separate functions for frame rate control.
---
drivers/media/i2c/ov7670.c | 136 ++--
1 file changed, 118 insertions(+), 18 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 3eaa06c..cb62d08 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -47,6 +47,8 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
*/
#define OV7670_I2C_ADDR 0x42
+#define PLL_FACTOR 4
+
/* Registers */
#define REG_GAIN 0x00/* Gain lower 8 bits (rest in vref) */
#define REG_BLUE 0x01/* blue gain */
@@ -164,6 +166,12 @@ MODULE_PARM_DESC(debug, "Debug level (0-1)");
#define REG_GFIX 0x69/* Fix gain control */
+#define REG_DBLV 0x6b/* PLL control an debugging */
+#define DBLV_BYPASS0x00/* Bypass PLL */
+#define DBLV_X40x01/* clock x4 */
+#define DBLV_X60x10/* clock x6 */
+#define DBLV_X80x11/* clock x8 */
+
#define REG_REG76 0x76/* OV's name */
#define R76_BLKPCOR0x80/* Black pixel correction enable */
#define R76_WHTPCOR0x40/* White pixel correction enable */
@@ -203,6 +211,9 @@ struct ov7670_devtype {
/* formats supported for each model */
struct ov7670_win_size *win_sizes;
unsigned int n_win_sizes;
+ /* callbacks for frame rate control */
+ int (*set_framerate)(struct v4l2_subdev *, struct v4l2_fract *);
+ void (*get_framerate)(struct v4l2_subdev *, struct v4l2_fract *);
};
/*
@@ -739,6 +750,98 @@ static struct ov7670_win_size ov7675_win_sizes[] = {
}
};
+static void ov7675_get_framerate(struct v4l2_subdev *sd,
+struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+ u32 clkrc = info->clkrc;
+ u32 pll_factor = PLL_FACTOR;
+
+ clkrc++;
+ if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
+ clkrc = (clkrc >> 1);
+
+ tpf->numerator = 1;
+ tpf->denominator = (5 * pll_factor * info->clock_speed) /
+ (4 * clkrc);
+}
+
+static int ov7675_set_framerate(struct v4l2_subdev *sd,
+struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+ u32 clkrc;
+ u32 pll_factor = PLL_FACTOR;
+ int ret;
+
+ /*
+* The formula is fps = 5/4*pixclk for YUV/RGB and
+* fps = 5/2*pixclk for RAW.
+*
+* pixclk = clock_speed / (clkrc + 1) * PLLfactor
+*
+*/
+ if (tpf->numerator == 0 || tpf->denominator == 0) {
+ clkrc = 0;
+ } else {
+ clkrc = (5 * pll_factor * info->clock_speed * tpf->numerator) /
+ (4 * tpf->denominator);
+ if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
+ clkrc = (clkrc << 1);
+ clkrc--;
+ }
+
+ /*
+* The datasheet claims that clkrc = 0 will divide the input clock by 1
+* but we've checked with an oscilloscope that it divides by 2 instead.
+* So, if clkrc = 0 just bypass the divider.
+*/
+ if (clkrc <= 0)
+ clkrc = CLK_EXT;
+ else if (clkrc > CLK_SCALE)
+ clkrc = CLK_SCALE;
+ info->clkrc = clkrc;
+
+ /* Recalculate frame rate */
+ ov7675_get_framerate(sd, tpf);
+
+ ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
+ if (ret < 0)
+ return ret;
+ return ov7670_write(sd, REG_DBLV, DBLV_X4);
+}
+
+static void ov7670_get_framerate_legacy(struct v4l2_subdev *sd,
+struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+
+ tpf->numerator = 1;
+ tpf->denominator = info->clock_speed;
+ if ((info->clkrc & CLK_EXT) == 0 && (info->clkrc & CLK_SCALE) > 1)
+ tpf->denominator /= (info->clkrc & CLK_SCALE);
+}
+
+static int ov7670_set_framerate_legacy(struct v4l2_subdev *sd,
+ struct v4l2_fract *tpf)
+{
+ struct ov7670_info *info = to_state(sd);
+ int div;
+
+ if (tpf->numerator == 0 || tpf->denominator == 0)
+ div = 1; /* Reset to full rate */
+ else
+ div = (tpf->numerator * info->clock_speed) / tpf->denominator;
+ if (div == 0)
+ div = 1;
+
[PATCH v2 2/5] media: ov7670: make try_fmt() consistent with 'min_height' and 'min_width'.
'min_height' and 'min_width' are variables that allow to specify the minimum
resolution that the sensor will achieve. This patch make v4l2 fmt callbacks
consider this parameters in order to return valid data to user space.
Acked-by: Jonathan Corbet
Signed-off-by: Javier Martin
---
drivers/media/i2c/ov7670.c | 22 +++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 17eb5d7..3eaa06c 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -786,10 +786,11 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
struct ov7670_format_struct **ret_fmt,
struct ov7670_win_size **ret_wsize)
{
- int index;
+ int index, i;
struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
unsigned int n_win_sizes = info->devtype->n_win_sizes;
+ unsigned int win_sizes_limit = n_win_sizes;
for (index = 0; index < N_OV7670_FMTS; index++)
if (ov7670_formats[index].mbus_code == fmt->code)
@@ -805,15 +806,30 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
* Fields: the OV devices claim to be progressive.
*/
fmt->field = V4L2_FIELD_NONE;
+
+ /*
+* Don't consider values that don't match min_height and min_width
+* constraints.
+*/
+ if (info->min_width || info->min_height)
+ for (i = 0; i < n_win_sizes; i++) {
+ wsize = info->devtype->win_sizes + i;
+
+ if (wsize->width < info->min_width ||
+ wsize->height < info->min_height) {
+ win_sizes_limit = i;
+ break;
+ }
+ }
/*
* Round requested image size down to the nearest
* we support, but not below the smallest.
*/
for (wsize = info->devtype->win_sizes;
-wsize < info->devtype->win_sizes + n_win_sizes; wsize++)
+wsize < info->devtype->win_sizes + win_sizes_limit; wsize++)
if (fmt->width >= wsize->width && fmt->height >= wsize->height)
break;
- if (wsize >= info->devtype->win_sizes + n_win_sizes)
+ if (wsize >= info->devtype->win_sizes + win_sizes_limit)
wsize--; /* Take the smallest one */
if (ret_wsize != NULL)
*ret_wsize = wsize;
--
1.7.9.5
--
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 v2 4/5] media: ov7670: add possibility to bypass pll for ov7675.
For a frame rate of 30 fps a pixclk of 24MHz is needed. For those
cases where the ov7670 has a clean 24MHz input (xvclk) the PLL
can be bypassed.
This will result in a value of clkrc of 1, which means that in practice
pixclk = xvclk (input clock)
Acked-by: Jonathan Corbet
Signed-off-by: Javier Martin
---
Changes since v1:
- Added changelog.
---
drivers/media/i2c/ov7670.c | 28 ++--
include/media/ov7670.h |1 +
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index cb62d08..559c23e 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -230,6 +230,7 @@ struct ov7670_info {
int clock_speed;/* External clock speed (MHz) */
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ bool pll_bypass;
const struct ov7670_devtype *devtype; /* Device specifics */
};
@@ -755,7 +756,12 @@ static void ov7675_get_framerate(struct v4l2_subdev *sd,
{
struct ov7670_info *info = to_state(sd);
u32 clkrc = info->clkrc;
- u32 pll_factor = PLL_FACTOR;
+ int pll_factor;
+
+ if (info->pll_bypass)
+ pll_factor = 1;
+ else
+ pll_factor = PLL_FACTOR;
clkrc++;
if (info->fmt->mbus_code == V4L2_MBUS_FMT_SBGGR8_1X8)
@@ -771,7 +777,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
{
struct ov7670_info *info = to_state(sd);
u32 clkrc;
- u32 pll_factor = PLL_FACTOR;
+ int pll_factor;
int ret;
/*
@@ -781,6 +787,16 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
* pixclk = clock_speed / (clkrc + 1) * PLLfactor
*
*/
+ if (info->pll_bypass) {
+ pll_factor = 1;
+ ret = ov7670_write(sd, REG_DBLV, DBLV_BYPASS);
+ } else {
+ pll_factor = PLL_FACTOR;
+ ret = ov7670_write(sd, REG_DBLV, DBLV_X4);
+ }
+ if (ret < 0)
+ return ret;
+
if (tpf->numerator == 0 || tpf->denominator == 0) {
clkrc = 0;
} else {
@@ -808,6 +824,7 @@ static int ov7675_set_framerate(struct v4l2_subdev *sd,
ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
if (ret < 0)
return ret;
+
return ov7670_write(sd, REG_DBLV, DBLV_X4);
}
@@ -1688,6 +1705,13 @@ static int ov7670_probe(struct i2c_client *client,
if (config->clock_speed)
info->clock_speed = config->clock_speed;
+
+ /*
+* It should be allowed for ov7670 too when it is migrated to
+* the new frame rate formula.
+*/
+ if (config->pll_bypass && id->driver_data != MODEL_OV7670)
+ info->pll_bypass = true;
}
/* Make sure it's an ov7670 */
diff --git a/include/media/ov7670.h b/include/media/ov7670.h
index b133bc1..a68c8bb 100644
--- a/include/media/ov7670.h
+++ b/include/media/ov7670.h
@@ -15,6 +15,7 @@ struct ov7670_config {
int min_height; /* Filter out smaller sizes */
int clock_speed;/* External clock speed (MHz) */
bool use_smbus; /* Use smbus I/O instead of I2C */
+ bool pll_bypass;/* Choose whether to bypass the PLL */
};
#endif
--
1.7.9.5
--
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 v2 5/5] media: ov7670: Add possibility to disable pixclk during hblank.
Some bridge drivers captures pixels during blanking periods if
pixclk is enabled. In order to avoid capturing bogus data we need to
disable pixclk in the sensor during those blanking periods.
Acked-by: Jonathan Corbet
Signed-off-by: Javier Martin
---
Changes since v1:
- Added changelog.
---
drivers/media/i2c/ov7670.c |7 +++
include/media/ov7670.h |1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 559c23e..9d7a92e 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -231,6 +231,7 @@ struct ov7670_info {
u8 clkrc; /* Clock divider value */
bool use_smbus; /* Use smbus I/O instead of I2C */
bool pll_bypass;
+ bool pclk_hb_disable;
const struct ov7670_devtype *devtype; /* Device specifics */
};
@@ -1712,6 +1713,9 @@ static int ov7670_probe(struct i2c_client *client,
*/
if (config->pll_bypass && id->driver_data != MODEL_OV7670)
info->pll_bypass = true;
+
+ if (config->pclk_hb_disable)
+ info->pclk_hb_disable = true;
}
/* Make sure it's an ov7670 */
@@ -1736,6 +1740,9 @@ static int ov7670_probe(struct i2c_client *client,
tpf.denominator = 30;
info->devtype->set_framerate(sd, &tpf);
+ if (info->pclk_hb_disable)
+ ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
+
return 0;
}
diff --git a/include/media/ov7670.h b/include/media/ov7670.h
index a68c8bb..1913d51 100644
--- a/include/media/ov7670.h
+++ b/include/media/ov7670.h
@@ -16,6 +16,7 @@ struct ov7670_config {
int clock_speed;/* External clock speed (MHz) */
bool use_smbus; /* Use smbus I/O instead of I2C */
bool pll_bypass;/* Choose whether to bypass the PLL */
+ bool pclk_hb_disable; /* Disable toggling pixclk during
horizontal blanking */
};
#endif
--
1.7.9.5
--
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
