Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size

[Hans Verkuil <hverkuil@xs4all.nl>]

On 04/27/2016 02:23 PM, Marek Szyprowski wrote:
> Hello,
> 
> 
> On 2016-04-27 14:10, Hans Verkuil wrote:
>> On 04/27/2016 02:00 PM, Marek Szyprowski wrote:
>>> Add a helper function for device drivers to set DMA's max_seg_size.
>>> Setting it to largest possible value lets DMA-mapping API always create
>>> contiguous mappings in DMA address space. This is essential for all
>>> devices, which use dma-contig videobuf2 memory allocator and shared
>>> buffers.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>> This patch was posted earlier as a part of
>>> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316
>>> thread, but applying it is really needed to get all Exynos multimedia
>>> drivers working with IOMMU enabled.
>>>
>>> Best regards,
>>> Marek Szyprowski
>>> ---
>>>   drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++++++++++++++
>>>   include/media/videobuf2-dma-contig.h           |  1 +
>>>   2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> index 5361197..f611456 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> @@ -753,6 +753,21 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx)
>>>   }
>>>   EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx);
>>>   +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size)
>>> +{
>>> +    if (!dev->dma_parms) {
>>> +        dev->dma_parms = devm_kzalloc(dev, sizeof(dev->dma_parms),
>>> +                          GFP_KERNEL);
>> The v3 patch from December uses kzalloc here. Is this perhaps on old version?
> 
> Right, my fault. I will do another resend (and fix the typo in the second patch).
> 
>>> +        if (!dev->dma_parms)
>>> +            return -ENOMEM;
>>> +    }
>>> +    if (dma_get_max_seg_size(dev) < size)
>>> +        return dma_set_max_seg_size(dev, size);
>>> +
>>> +    return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size);
>> Admittedly I haven't looked closely at this, but is this something that you
>> want for all dma-contig devices? Or to rephrase this question: what type of
>> devices will need this?
> 
> This is needed for all devices using vb2-dc, iommu and user-ptr mode, however
> in the previous discussions (see https://patchwork.linuxtv.org/patch/30870/
> ) it has been suggested to make it via common helper instead of forcing it
> in vb2-dc.

This certainly will need to be carefully documented in videobuf2-dma-contig.h.

What happens if it is called when you don't have an iommu? Does something break?

Regards,

	Hans