Hi Marek, On 05/04/2016 10:28 AM, Marek Szyprowski wrote: > Hi Hans, > > > On 2016-05-04 10:22, Hans Verkuil wrote: >> Hi Marek, >> >> On 05/02/2016 12:59 PM, Marek Szyprowski wrote: >>> This patch lets vb2-dma-contig memory allocator to configure DMA max >>> segment size properly for the client device. Setting it is needed to let >>> DMA-mapping subsystem to create a single, contiguous mapping in DMA >>> address space. This is essential for all devices, which use dma-contig >>> videobuf2 memory allocator and shared buffers (in USERPTR or DMAbuf modes >>> of operations). >>> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >>> --- >>> Hello, >>> >>> This patch is a follow-up of my previous attempts to let Exynos >>> multimedia devices to work properly with shared buffers when IOMMU is >>> enabled: >>> 1. https://www.mail-archive.com/linux-media@xxxxxxxxxxxxxxx/msg96946.html >>> 2. http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/97316 >>> 3. https://patchwork.linuxtv.org/patch/30870/ >>> >>> As sugested by Hans, configuring DMA max segment size should be done by >>> videobuf2-dma-contig module instead of requiring all device drivers to >>> do it on their own. >>> >>> Here is some backgroud why this is done in videobuf2-dc not in the >>> respective generic bus code: >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html >>> >>> Best regards, >>> Marek Szyprowski >>> >>> changelog: >>> v3: >>> - added FIXME note about possible memory leak >>> >>> v2: >>> - fixes typos and other language issues in the comments >>> >>> v1: http://article.gmane.org/gmane.linux.kernel.samsung-soc/53690 >>> --- >>> drivers/media/v4l2-core/videobuf2-dma-contig.c | 45 ++++++++++++++++++++++++++ >>> 1 file changed, 45 insertions(+) >>> >>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c >>> index 461ae55eaa98..2ca7e798f394 100644 >>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c >>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c >>> @@ -443,6 +443,42 @@ static void vb2_dc_put_userptr(void *buf_priv) >>> } >>> >>> /* >>> + * To allow mapping the scatter-list into a single chunk in the DMA >>> + * address space, the device is required to have the DMA max segment >>> + * size parameter set to a value larger than the buffer size. Otherwise, >>> + * the DMA-mapping subsystem will split the mapping into max segment >>> + * size chunks. This function increases the DMA max segment size >>> + * parameter to let DMA-mapping map a buffer as a single chunk in DMA >>> + * address space. >>> + * This code assumes that the DMA-mapping subsystem will merge all >>> + * scatterlist segments if this is really possible (for example when >>> + * an IOMMU is available and enabled). >>> + * Ideally, this parameter should be set by the generic bus code, but it >>> + * is left with the default 64KiB value due to historical litmiations in >>> + * other subsystems (like limited USB host drivers) and there no good >>> + * place to set it to the proper value. It is done here to avoid fixing >>> + * all the vb2-dc client drivers. >>> + * >>> + * FIXME: the allocated dma_params structure is leaked because there >>> + * is completely no way to determine when to free it (dma_params might have >>> + * been also already allocated by the bus code). However in typical >>> + * use cases this function will be called for platform devices, which are >>> + * not how-plugged and exist all the time in the target system. >>> + */ >>> +static int vb2_dc_set_max_seg_size(struct device *dev, unsigned int size) >>> +{ >>> + if (!dev->dma_parms) { >>> + dev->dma_parms = kzalloc(sizeof(dev->dma_parms), GFP_KERNEL); >>> + if (!dev->dma_parms) >>> + return -ENOMEM; >>> + } >>> + if (dma_get_max_seg_size(dev) < size) >>> + return dma_set_max_seg_size(dev, size); >>> + >>> + return 0; >>> +} >>> + >>> +/* >>> * For some kind of reserved memory there might be no struct page available, >>> * so all that can be done to support such 'pages' is to try to convert >>> * pfn to dma address or at the last resort just assume that >>> @@ -499,6 +535,10 @@ static void *vb2_dc_get_userptr(struct device *dev, unsigned long vaddr, >>> return ERR_PTR(-EINVAL); >>> } >>> >>> + ret = vb2_dc_set_max_seg_size(dev, PAGE_ALIGN(size + PAGE_SIZE)); >> Huh? Against which kernel do you compile? The get_userptr prototype is different >> from the latest mainline kernel. Specifically, dev is now conf->dev. > > I prepared it on top of your 'context3' branch, as you requested not to > use the > allocator context related functions, which best suit for this purpose. That's not quite what I meant, sorry for the confusion. My reference to the context3 branch was just that: to show upcoming changes and why it was not a good idea to call this function from the context allocate/free functions. Since those will disappear. The context3 branch isn't for 4.7 (too late for that), but I want to get it in early in the 4.8 cycle. So just base this patch on the latest media_tree master. Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html