On Tue, 2016-02-16 at 14:48 +0100, Hans Verkuil wrote:
> Hi Tiffany,
>
> >>>>> +int mtk_vcodec_enc_queue_init(void *priv, struct vb2_queue *src_vq,
> >>>>> + struct vb2_queue *dst_vq)
> >>>>> +{
> >>>>> + struct mtk_vcodec_ctx *ctx = priv;
> >>>>> + int ret;
> >>>>> +
> >>>>> + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> >>>>> + src_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
> >>>>
> >>>> I recomment dropping VB2_USERPTR. That only makes sense for
> >>>> scatter-gather dma,
> >>>> and you use physically contiguous DMA.
> >>>>
> >>> Now our userspace app use VB2_USERPTR. I need to check if we could drop
> >>> VB2_USERPTR.
> >>> We use src_vq->mem_ops = &vb2_dma_contig_memops;
> >>> And there are
> >>> .get_userptr = vb2_dc_get_userptr,
> >>> .put_userptr = vb2_dc_put_userptr,
> >>> I was confused why it only make sense for scatter-gather.
> >>> Could you kindly explain more?
> >>
> >> VB2_USERPTR indicates that the application can use malloc to allocate
> >> buffers
> >> and pass those to the driver. Since malloc uses virtual memory the physical
> >> memory is scattered all over. And the first page typically does not start
> >> at
> >> the beginning of the page but at a random offset.
> >>
> >> To support that the DMA generally has to be able to do scatter-gather.
> >>
> >> Now, where things get ugly is that a hack was added to the USERPTR support
> >> where
> >> apps could pass a pointer to physically contiguous memory as a user
> >> pointer. This
> >> was a hack for embedded systems that preallocated a pool of buffers and
> >> needed to
> >> pass those pointers around somehow. So the dma-contig USERPTR support is
> >> for that
> >> 'feature'. If you try to pass a malloc()ed buffer to a dma-contig driver
> >> it will
> >> reject it. One big problem is that this specific hack isn't signaled
> >> anywhere, so
> >> applications have no way of knowing if the USERPTR support is the proper
> >> version
> >> or the hack where physically contiguous memory is expected.
> >>
> >> This hack has been replaced with DMABUF which is the proper way of passing
> >> buffers
> >> around.
> >>
> >> New dma-contig drivers should not use that old hack anymore. Use dmabuf to
> >> pass
> >> external buffers around.
> >>
> >> How do you use it in your app? With malloc()ed buffers? Or with 'special'
> >> pointers
> >> to physically contiguous buffers?
> >>
> > Understood now. Thanks for your explanation.
> > Now our app use malloc()ed buffers and we hook vb2_dma_contig_memops.
> > I don't know why that dma-contig driver do not reject it.
> > I will try to figure it out.
>
> Is there an iommu involved that turns the scatter-gather list into what looks
> like
> contiguous memory for the DMA?
>
Yes, We have iommu that could make scatter-gather list looks like
contiguous memory.
> At the end of vb2_dc_get_userptr() in videobuf2-dma-contig.c there is a check
> 'if (contig_size < size)' that verifies that the sg DMA is contiguous. This
> would
> work if there is an iommu involved (if I understand it correctly).
>
I see. We saw this error before we add iommu support.
> If that's the case, then it's OK to keep VB2_USERPTR because you have real sg
> support (although not via the DMA engine, but via iommu mappings).
>
Got it. We will keep VB2_USERPTR.
> Regards,
>
> Hans
