Re: [PATCH v2] media: videobuf2-dc: set properly dma_max_segment_size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Aug 13, 2015 at 7:49 AM, Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
> Hello,
>
>
> On 2015-08-13 14:40, Lars-Peter Clausen wrote:
>>
>> On 08/12/2015 11:58 AM, Marek Szyprowski wrote:
>>>
>>> If device has no DMA max_seg_size set, we assume that there is no limit
>>> and it is safe to force it to use DMA_BIT_MASK(32) as max_seg_size to
>>> let DMA-mapping API always create contiguous mappings in DMA address
>>> space. This is essential for all devices, which use dma-contig
>>> videobuf2 memory allocator.
>>>
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
>>> ---
>>> Changelog:
>>> v2:
>>> - set max segment size only if a new dma params structure has been
>>>    allocated, as suggested by Laurent Pinchart
>>> ---
>>>   drivers/media/v4l2-core/videobuf2-dma-contig.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> index 94c1e64..455e925 100644
>>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>>> @@ -862,6 +862,21 @@ EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
>>>   void *vb2_dma_contig_init_ctx(struct device *dev)
>>>   {
>>>         struct vb2_dc_conf *conf;
>>> +       int err;
>>> +
>>> +       /*
>>> +        * if device has no max_seg_size set, we assume that there is no
>>> limit
>>> +        * and force it to DMA_BIT_MASK(32) to always use contiguous
>>> mappings
>>> +        * in DMA address space
>>> +        */
>>> +       if (!dev->dma_parms) {
>>> +               dev->dma_parms = kzalloc(sizeof(*dev->dma_parms),
>>> GFP_KERNEL);
>>> +               if (!dev->dma_parms)
>>> +                       return ERR_PTR(-ENOMEM);
>>> +               err = dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
>>> +               if (err)
>>> +                       return ERR_PTR(err);
>>> +       }
>>
>> I'm not sure if this is such a good idea. The DMA provider is responsible
>> for setting this up. We shouldn't be overwriting this here on the DMA
>> consumer side. This will just mask the bug that the provider didn't setup
>> this correctly and might cause bugs on its own if it is not correct. It
>> will
>> lead to conflicts with DMA providers that have multiple consumers (e.g.
>> shared DMA core). And also the current assumption is that if a driver
>> doesn't set this up explicitly the maximum segement size is 65536.
>
>
> The problem is that there is no good place for changing this extremely low
> default
> value. V4L2 media devices, which use videobuf2-dc expects to get buffers
> mapped
> contiguous in the DMA/IO address space. Initially I wanted to have a code
> for
> setting dma max segment size directly in the dma-mapping subsystem. This
> however
> causeed problems in the other places, as mentioned in the following mail:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html
>
> It looks that there are drivers or subsystems which rely on this strange 64k
> value,
> rending the whole concept rather useless.
>

Would this approach make the driver not work on some architectures? Very
few drives tweak this value. I found 19 calls to the dma_set_max_seg_size()
functions directly and a handful via pci_set_dma_max_seg_size().

Is this one those, "use if you absolutely have to" interfaces?

thanks,
-- Shuah
--
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



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux