On Thu, 23 Sep 2010, Janusz Krzysztofik wrote:
> Thursday 23 September 2010 15:33:54 Guennadi Liakhovetski napisaÅ(a):
> > On Wed, 22 Sep 2010, Janusz Krzysztofik wrote:
> > > Wednesday 22 September 2010 01:23:22 Guennadi Liakhovetski napisaÅ(a):
> > > > On Sat, 11 Sep 2010, Janusz Krzysztofik wrote:
> > > > > +
> > > > > + vb = &buf->vb;
> > > > > + if (waitqueue_active(&vb->done)) {
> > > > > + if (!pcdev->ready && result != VIDEOBUF_ERROR)
> > > > > + /*
> > > > > + * No next buffer has been entered into the DMA
> > > > > + * programming register set on time, so best we can do
> > > > > + * is stopping the capture before last DMA block,
> > > > > + * whether our CONTIG mode whole buffer or its last
> > > > > + * sgbuf in SG mode, gets overwritten by next frame.
> > > > > + */
> > > >
> > > > Hm, why do you think it's a good idea? This specific buffer completed
> > > > successfully, but you want to fail it just because the next buffer is
> > > > missing? Any specific reason for this?
> > >
> > > Maybe my comment is not clear enough, but the below suspend_capture()
> > > doesn't indicate any failure on a frame just captured. It only prevents
> > > the frame from being overwritten by the already autoreinitialized DMA
> > > engine, pointing back to the same buffer once again.
> > >
> > > > Besides, you seem to also be
> > > > considering the possibility of your ->ready == NULL, but the queue
> > > > non-empty, in which case you just take the next buffer from the queue
> > > > and continue with it. Why error out in this case?
> > >
> > > pcdev->ready == NULL means no buffer was available when it was time to
> > > put it into the DMA programming register set.
> >
> > But how? Buffers are added onto the list in omap1_videobuf_queue() under
> > spin_lock_irqsave(); and there you also check ->ready and fill it in.
>
> Guennadi,
> Yes, but only if pcdev->active is NULL, ie. both DMA and FIFO are idle, never
> if active:
>
> + list_add_tail(&vb->queue, &pcdev->capture);
> + vb->state = VIDEOBUF_QUEUED;
> +
> + if (pcdev->active)
> + return;
>
> Since the transfer of the DMA programming register set content to the DMA
> working register set is done automatically by the DMA hardware, this can
> pretty well happen while I keep the lock here, so I can't be sure if it's not
> too late for entering new data into the programming register set. Then, I
> decided that this operation should be done only just after the DMA interrupt
> occured, ie. the current DMA programming register set content has just been
> used and can be overwriten.
>
> I'll emphasize the above return; with a comment.
Ok
> > In
> > your completion you set ->ready = NULL, but then also call
> > prepare_next_vb() to get the next buffer from the list - if there are any,
> > so, how can it be NULL with a non-empty list?
>
> It happens after the above mentioned prepare_next_vb() gets nothing from an
> empty queue, so nothing is entered into the DMA programming register set,
> only the last, just activated, buffer is processed, then
> omap1_videobuf_queue() puts a new buffer into the queue while the active
> buffer is still filled in, and finally the DMA ISR is called on this last
> active buffer completion.
>
> I hope this helps.
Let's assume it does:) You seem to really understand how this is working
and even be willing to document the driver, thus making it possibly the
best documented soc-camera related piece of software;)
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
