From: hverkuil@xs4all•nl (Hans Verkuil)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v4 5/8] [Media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver
Date: Wed, 17 Feb 2016 09:31:31 +0100 [thread overview]
Message-ID: <56C42FE3.8000105@xs4all.nl> (raw)
In-Reply-To: <1455696068.26202.4.camel@mtksdaap41>
On 02/17/16 09:01, tiffany lin wrote:
> 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.
Can you add a comment here mentioning that VB2_USERPTR works with dma-contig because
there is an iommu? That should clarify this.
Regards,
Hans
next prev parent reply other threads:[~2016-02-17 8:31 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-04 11:34 [PATCH v4 0/8] Add MT8173 Video Encoder Driver and VPU Driver Tiffany Lin
2016-02-04 11:34 ` [PATCH v4 1/8] dt-bindings: Add a binding for Mediatek Video Processor Tiffany Lin
2016-02-04 11:34 ` [PATCH v4 2/8] [media] VPU: mediatek: support Mediatek VPU Tiffany Lin
2016-02-04 11:34 ` [PATCH v4 3/8] arm64: dts: mediatek: Add node for Mediatek Video Processor Unit Tiffany Lin
2016-02-04 11:34 ` [PATCH v4 4/8] dt-bindings: Add a binding for Mediatek Video Encoder Tiffany Lin
[not found] ` <1454585703-42428-6-git-send-email-tiffany.lin@mediatek.com>
2016-02-04 11:35 ` [PATCH v4 6/8] [media] vcodec: mediatek: Add Mediatek VP8 Video Encoder Driver Tiffany Lin
2016-02-04 11:35 ` [PATCH v4 7/8] [media] vcodec: mediatek: Add Mediatek H264 " Tiffany Lin
2016-02-04 11:35 ` [PATCH v4 8/8] arm64: dts: mediatek: Add Video Encoder for MT8173 Tiffany Lin
2016-02-15 11:33 ` [PATCH v4 7/8] [media] vcodec: mediatek: Add Mediatek H264 Video Encoder Driver Hans Verkuil
2016-02-16 11:57 ` pochun lin
[not found] ` <56C1B4AF.1030207@xs4all.nl>
[not found] ` <1455604653.19396.68.camel@mtksdaap41>
2016-02-16 7:44 ` [PATCH v4 5/8] [Media] vcodec: mediatek: Add Mediatek V4L2 " Hans Verkuil
2016-02-16 13:20 ` tiffany lin
2016-02-16 13:48 ` Hans Verkuil
2016-02-17 8:01 ` tiffany lin
2016-02-17 8:31 ` Hans Verkuil [this message]
2016-02-17 9:23 ` tiffany lin
2016-02-20 9:11 ` tiffany lin
2016-02-20 9:18 ` Hans Verkuil
2016-02-22 15:19 ` tiffany lin
2016-02-17 7:47 ` Hans Verkuil
2016-02-17 8:33 ` tiffany lin
2016-02-08 18:56 ` [PATCH v4 4/8] dt-bindings: Add a binding for Mediatek Video Encoder Rob Herring
2016-02-09 11:29 ` Daniel Kurtz
2016-02-15 10:42 ` Daniel Kurtz
2016-02-16 2:09 ` tiffany lin
2016-02-15 10:07 ` [PATCH v4 2/8] [media] VPU: mediatek: support Mediatek VPU Hans Verkuil
2016-02-15 11:20 ` tiffany lin
2016-02-15 10:13 ` Hans Verkuil
2016-02-15 11:27 ` tiffany lin
2016-02-15 13:59 ` Wu-Cheng Li (李務誠)
2016-02-16 9:36 ` andrew-ct chen
2016-02-08 18:54 ` [PATCH v4 1/8] dt-bindings: Add a binding for Mediatek Video Processor Rob Herring
2016-02-15 12:04 ` [PATCH v4 0/8] Add MT8173 Video Encoder Driver and VPU Driver Hans Verkuil
2016-02-16 6:46 ` tiffany lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56C42FE3.8000105@xs4all.nl \
--to=hverkuil@xs4all$(echo .)nl \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox