On Mon, Jan 18, 2016 at 7:45 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> On 01/18/2016 05:26 PM, Ran Shalit wrote:
>> Hello,
>>
>> I am trying to understand how to implement dma transfer correctly
>> using videobuf2 APIs.
>>
>> Normally, application will do semthing like this (video test API):
>>
>> xioctl(fd, VIDIOC_DQBUF, &buf)
>> process_image(buffers[buf.index].start, buf.bytesused);
>> xioctl(fd, VIDIOC_QBUF, &buf)
>>
>> Therefore, in the driver below I will assume that:
>> 1. VIDIOC_DQBUF - trigger dma to start
>
> No. DMA typically starts when VIDIOC_STREAMON is called, although depending
> on the hardware the start of the DMA may be delayed if insufficient buffers
> have been queued. When the start_streaming op is called you know that STREAMON
> has been called and that at least min_buffers_needed buffers have been queued.
>
>> 2. interrupt handler in driver - stop dma
>
> ??? No, this just passes the buffer that has been filled by the DMA engine
> to vb2 via vb2_buffer_done. The DMA will continue filling the next queued buffer.
>
Hi,
Just to be sure I got it all right:
"The DMA will continue filling the next queued buffer" is usually the
responsibility of the interrupt handler .
Interrrupt will get the new buffer from list and trigger next dma
transaction with that buffer.
Is that Right ?
Thank you all very much for the time,
Ran
>> 3. VIDIOC_QBUF - do nothing with dma.
>>
>> But, on code review of the following two drivers, I see other things,
>> much more complex, and I don't understand it yet.
>>
>> These are the two drivers I reviewed:
>> - STA2X11
>> - dt3511
>>
>> In STA2X11 I see:
>>
>> 1. start_streaming - also triggers dma to start, why ?
>
> See above, that's what start_streaming typically does.
>
>> 2. buf_queue - add buffer to list & if No active buffer, active the
>> first one , and trigger dma. why do we trigger dma with buf_queue
>
> This is old code: if start_streaming is called when there are insufficient
> buffers, then start_streaming won't call start_dma and this is postponed
> until buf_queue is called and the minimum number of buffers is reached.
>
> These days you'd set the min_buffers_needed field (like in dt3155), and
> always start the dma engine in start_streaming. This feature didn't exist
> when this driver was written.
>
>> (I
>> would assume triggering is done with VIDIOC_DQBUF -> buffer_finish) ?
>> 3. buf_finish - remove buffer from list, but also get the next buffer
>> in list and trigger dma, why do we need to trigger a next buffer ?
>> Isn't buffer_finish is called as a result of VIDIOC_QBUF ?
>
> I don't think this in done in the right place. I really wouldn't use sta2x11
> as a template. It's poorly maintained.
>
>>
>>
>> In dt3511 I see something else as following:
>> buf_queue()
>> {...
>> if (pd->curr_buf)
>> list_add_tail(&vb->done_entry, &pd->dmaq);
>> else {
>> pd->curr_buf = vb;
>> elbit_start_acq(pd);
>> }
>> ...}
>>
>> 1. Again, why dma triggering is done as part of buf_queue instead buf_finish
>
> ?? It doesn't. This is the code from the latest kernel:
>
> static void dt3155_buf_queue(struct vb2_buffer *vb)
> {
> struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> struct dt3155_priv *pd = vb2_get_drv_priv(vb->vb2_queue);
>
> /* pd->vidq.streaming = 1 when dt3155_buf_queue() is invoked */
> spin_lock_irq(&pd->lock);
> if (pd->curr_buf)
> list_add_tail(&vb->done_entry, &pd->dmaq);
> else
> pd->curr_buf = vbuf;
> spin_unlock_irq(&pd->lock);
> }
>
> Are you looking at the dt3155 driver from some old kernel? I've no idea
> where you get that code from. BTW, it's dt3155, not dt3511.
>
>> 2. what's the meaning of the condition in this code, it is as if only
>> the first buffer in buf_queue i striggered, what about the next
>> ones,why do they only get into list without triggering dma ?
>> 3. In this driver there is no buf_finish.
>
> What each vb2_op does is documented in the comment before struct vb2_ops in
> videobuf2-core.h. This includes which ops are optional and which aren't
> (e.g. buf_finish is optional: if there is nothing to be done there you can
> leave it out).
>
> Since there are so many different DMA engines, each with its own peculiarities,
> what happens in which op can differ.
>
> Regards,
>
> Hans
>
>>
>> Thank you for any comments,
>> Ran
>> --
>> 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
>>
>
--
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
