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 <javier.martin@xxxxxxxxxxxxxxxxx> > --- > 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; 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:-) > + > + 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, Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html