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. > 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