* [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size @ 2016-04-27 12:00 Marek Szyprowski 2016-04-27 12:00 ` [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs Marek Szyprowski 2016-04-27 12:10 ` [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Hans Verkuil 0 siblings, 2 replies; 8+ messages in thread From: Marek Szyprowski @ 2016-04-27 12:00 UTC (permalink / raw) To: linux-media, linux-samsung-soc Cc: Marek Szyprowski, Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz 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); + 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); + MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2"); MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>"); MODULE_LICENSE("GPL"); diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h index 2087c9a..98857b5 100644 --- a/include/media/videobuf2-dma-contig.h +++ b/include/media/videobuf2-dma-contig.h @@ -35,6 +35,7 @@ static inline void *vb2_dma_contig_init_ctx(struct device *dev) } void vb2_dma_contig_cleanup_ctx(void *alloc_ctx); +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size); extern const struct vb2_mem_ops vb2_dma_contig_memops; -- 1.9.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs 2016-04-27 12:00 [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Marek Szyprowski @ 2016-04-27 12:00 ` Marek Szyprowski 2016-04-27 12:08 ` Hans Verkuil 2016-04-27 12:10 ` [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Hans Verkuil 1 sibling, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2016-04-27 12:00 UTC (permalink / raw) To: linux-media, linux-samsung-soc Cc: Marek Szyprowski, Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz All multimedia devices found on Exynos SoCs support only contiguous buffers, so set DMA max segment size to DMA_BIT_MASK(32) to let memory allocator to correctly create contiguous memory mappings. 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/platform/exynos-gsc/gsc-core.c | 1 + drivers/media/platform/exynos4-is/fimc-core.c | 1 + drivers/media/platform/exynos4-is/fimc-is.c | 1 + drivers/media/platform/exynos4-is/fimc-lite.c | 1 + drivers/media/platform/s5p-g2d/g2d.c | 1 + drivers/media/platform/s5p-jpeg/jpeg-core.c | 1 + drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++ drivers/media/platform/s5p-tv/mixer_video.c | 1 + 8 files changed, 9 insertions(+) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index 9b9e423..4f90be4 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1140,6 +1140,7 @@ static int gsc_probe(struct platform_device *pdev) goto err_m2m; /* Initialize continious memory allocator */ + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); gsc->alloc_ctx = vb2_dma_contig_init_ctx(dev); if (IS_ERR(gsc->alloc_ctx)) { ret = PTR_ERR(gsc->alloc_ctx); diff --git a/drivers/media/platform/exynos4-is/fimc-core.c b/drivers/media/platform/exynos4-is/fimc-core.c index cef2a7f..368e19b 100644 --- a/drivers/media/platform/exynos4-is/fimc-core.c +++ b/drivers/media/platform/exynos4-is/fimc-core.c @@ -1019,6 +1019,7 @@ static int fimc_probe(struct platform_device *pdev) } /* Initialize contiguous memory allocator */ + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev); if (IS_ERR(fimc->alloc_ctx)) { ret = PTR_ERR(fimc->alloc_ctx); diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c index 979c388..3f50856 100644 --- a/drivers/media/platform/exynos4-is/fimc-is.c +++ b/drivers/media/platform/exynos4-is/fimc-is.c @@ -847,6 +847,7 @@ static int fimc_is_probe(struct platform_device *pdev) if (ret < 0) goto err_pm; + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); is->alloc_ctx = vb2_dma_contig_init_ctx(dev); if (IS_ERR(is->alloc_ctx)) { ret = PTR_ERR(is->alloc_ctx); diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c index dc1b929..95841c8 100644 --- a/drivers/media/platform/exynos4-is/fimc-lite.c +++ b/drivers/media/platform/exynos4-is/fimc-lite.c @@ -1551,6 +1551,7 @@ static int fimc_lite_probe(struct platform_device *pdev) goto err_sd; } + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev); if (IS_ERR(fimc->alloc_ctx)) { ret = PTR_ERR(fimc->alloc_ctx); diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c index 74bd46c..5048b68 100644 --- a/drivers/media/platform/s5p-g2d/g2d.c +++ b/drivers/media/platform/s5p-g2d/g2d.c @@ -681,6 +681,7 @@ static int g2d_probe(struct platform_device *pdev) goto put_clk_gate; } + vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev); if (IS_ERR(dev->alloc_ctx)) { ret = PTR_ERR(dev->alloc_ctx); diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index c3b13a6..e535ccf 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -2838,6 +2838,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev) goto device_register_rollback; } + vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); jpeg->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev); if (IS_ERR(jpeg->alloc_ctx)) { v4l2_err(&jpeg->v4l2_dev, "Failed to init memory allocator\n"); diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index 927ab49..ae0bf26 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -1164,11 +1164,13 @@ static int s5p_mfc_probe(struct platform_device *pdev) } } + vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32)); dev->alloc_ctx[0] = vb2_dma_contig_init_ctx(dev->mem_dev_l); if (IS_ERR(dev->alloc_ctx[0])) { ret = PTR_ERR(dev->alloc_ctx[0]); goto err_res; } + vb2_dma_contig_set_max_seg_size(dev->mem_dev_r, DMA_BIT_MASK(32)); dev->alloc_ctx[1] = vb2_dma_contig_init_ctx(dev->mem_dev_r); if (IS_ERR(dev->alloc_ctx[1])) { ret = PTR_ERR(dev->alloc_ctx[1]); diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c index d9e7f03..17d6df7 100644 --- a/drivers/media/platform/s5p-tv/mixer_video.c +++ b/drivers/media/platform/s5p-tv/mixer_video.c @@ -80,6 +80,7 @@ int mxr_acquire_video(struct mxr_device *mdev, goto fail; } + vb2_dma_contig_set_max_seg_size(mdev->dev, DMA_BIT_MASK(32)); mdev->alloc_ctx = vb2_dma_contig_init_ctx(mdev->dev); if (IS_ERR(mdev->alloc_ctx)) { mxr_err(mdev, "could not acquire vb2 allocator\n"); -- 1.9.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs 2016-04-27 12:00 ` [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs Marek Szyprowski @ 2016-04-27 12:08 ` Hans Verkuil 0 siblings, 0 replies; 8+ messages in thread From: Hans Verkuil @ 2016-04-27 12:08 UTC (permalink / raw) To: Marek Szyprowski, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz On 04/27/2016 02:00 PM, Marek Szyprowski wrote: > All multimedia devices found on Exynos SoCs support only contiguous > buffers, so set DMA max segment size to DMA_BIT_MASK(32) to let memory > allocator to correctly create contiguous memory mappings. > > 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/platform/exynos-gsc/gsc-core.c | 1 + > drivers/media/platform/exynos4-is/fimc-core.c | 1 + > drivers/media/platform/exynos4-is/fimc-is.c | 1 + > drivers/media/platform/exynos4-is/fimc-lite.c | 1 + > drivers/media/platform/s5p-g2d/g2d.c | 1 + > drivers/media/platform/s5p-jpeg/jpeg-core.c | 1 + > drivers/media/platform/s5p-mfc/s5p_mfc.c | 2 ++ > drivers/media/platform/s5p-tv/mixer_video.c | 1 + > 8 files changed, 9 insertions(+) > > diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c > index 9b9e423..4f90be4 100644 > --- a/drivers/media/platform/exynos-gsc/gsc-core.c > +++ b/drivers/media/platform/exynos-gsc/gsc-core.c > @@ -1140,6 +1140,7 @@ static int gsc_probe(struct platform_device *pdev) > goto err_m2m; > > /* Initialize continious memory allocator */ Typo: continious -> contiguous Regards, Hans > + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > gsc->alloc_ctx = vb2_dma_contig_init_ctx(dev); > if (IS_ERR(gsc->alloc_ctx)) { > ret = PTR_ERR(gsc->alloc_ctx); > diff --git a/drivers/media/platform/exynos4-is/fimc-core.c b/drivers/media/platform/exynos4-is/fimc-core.c > index cef2a7f..368e19b 100644 > --- a/drivers/media/platform/exynos4-is/fimc-core.c > +++ b/drivers/media/platform/exynos4-is/fimc-core.c > @@ -1019,6 +1019,7 @@ static int fimc_probe(struct platform_device *pdev) > } > > /* Initialize contiguous memory allocator */ > + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev); > if (IS_ERR(fimc->alloc_ctx)) { > ret = PTR_ERR(fimc->alloc_ctx); > diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c > index 979c388..3f50856 100644 > --- a/drivers/media/platform/exynos4-is/fimc-is.c > +++ b/drivers/media/platform/exynos4-is/fimc-is.c > @@ -847,6 +847,7 @@ static int fimc_is_probe(struct platform_device *pdev) > if (ret < 0) > goto err_pm; > > + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > is->alloc_ctx = vb2_dma_contig_init_ctx(dev); > if (IS_ERR(is->alloc_ctx)) { > ret = PTR_ERR(is->alloc_ctx); > diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c > index dc1b929..95841c8 100644 > --- a/drivers/media/platform/exynos4-is/fimc-lite.c > +++ b/drivers/media/platform/exynos4-is/fimc-lite.c > @@ -1551,6 +1551,7 @@ static int fimc_lite_probe(struct platform_device *pdev) > goto err_sd; > } > > + vb2_dma_contig_set_max_seg_size(dev, DMA_BIT_MASK(32)); > fimc->alloc_ctx = vb2_dma_contig_init_ctx(dev); > if (IS_ERR(fimc->alloc_ctx)) { > ret = PTR_ERR(fimc->alloc_ctx); > diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c > index 74bd46c..5048b68 100644 > --- a/drivers/media/platform/s5p-g2d/g2d.c > +++ b/drivers/media/platform/s5p-g2d/g2d.c > @@ -681,6 +681,7 @@ static int g2d_probe(struct platform_device *pdev) > goto put_clk_gate; > } > > + vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); > dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev); > if (IS_ERR(dev->alloc_ctx)) { > ret = PTR_ERR(dev->alloc_ctx); > diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c > index c3b13a6..e535ccf 100644 > --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c > +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c > @@ -2838,6 +2838,7 @@ static int s5p_jpeg_probe(struct platform_device *pdev) > goto device_register_rollback; > } > > + vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32)); > jpeg->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev); > if (IS_ERR(jpeg->alloc_ctx)) { > v4l2_err(&jpeg->v4l2_dev, "Failed to init memory allocator\n"); > diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c > index 927ab49..ae0bf26 100644 > --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c > +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c > @@ -1164,11 +1164,13 @@ static int s5p_mfc_probe(struct platform_device *pdev) > } > } > > + vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32)); > dev->alloc_ctx[0] = vb2_dma_contig_init_ctx(dev->mem_dev_l); > if (IS_ERR(dev->alloc_ctx[0])) { > ret = PTR_ERR(dev->alloc_ctx[0]); > goto err_res; > } > + vb2_dma_contig_set_max_seg_size(dev->mem_dev_r, DMA_BIT_MASK(32)); > dev->alloc_ctx[1] = vb2_dma_contig_init_ctx(dev->mem_dev_r); > if (IS_ERR(dev->alloc_ctx[1])) { > ret = PTR_ERR(dev->alloc_ctx[1]); > diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c > index d9e7f03..17d6df7 100644 > --- a/drivers/media/platform/s5p-tv/mixer_video.c > +++ b/drivers/media/platform/s5p-tv/mixer_video.c > @@ -80,6 +80,7 @@ int mxr_acquire_video(struct mxr_device *mdev, > goto fail; > } > > + vb2_dma_contig_set_max_seg_size(mdev->dev, DMA_BIT_MASK(32)); > mdev->alloc_ctx = vb2_dma_contig_init_ctx(mdev->dev); > if (IS_ERR(mdev->alloc_ctx)) { > mxr_err(mdev, "could not acquire vb2 allocator\n"); > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size 2016-04-27 12:00 [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Marek Szyprowski 2016-04-27 12:00 ` [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs Marek Szyprowski @ 2016-04-27 12:10 ` Hans Verkuil 2016-04-27 12:23 ` Marek Szyprowski 1 sibling, 1 reply; 8+ messages in thread From: Hans Verkuil @ 2016-04-27 12:10 UTC (permalink / raw) To: Marek Szyprowski, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz 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? > + 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? Regards, Hans > + > MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2"); > MODULE_AUTHOR("Pawel Osciak <pawel@osciak.com>"); > MODULE_LICENSE("GPL"); > diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h > index 2087c9a..98857b5 100644 > --- a/include/media/videobuf2-dma-contig.h > +++ b/include/media/videobuf2-dma-contig.h > @@ -35,6 +35,7 @@ static inline void *vb2_dma_contig_init_ctx(struct device *dev) > } > > void vb2_dma_contig_cleanup_ctx(void *alloc_ctx); > +int vb2_dma_contig_set_max_seg_size(struct device *dev, unsigned int size); > > extern const struct vb2_mem_ops vb2_dma_contig_memops; > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size 2016-04-27 12:10 ` [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Hans Verkuil @ 2016-04-27 12:23 ` Marek Szyprowski 2016-04-27 12:58 ` Hans Verkuil 0 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2016-04-27 12:23 UTC (permalink / raw) To: Hans Verkuil, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz 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. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size 2016-04-27 12:23 ` Marek Szyprowski @ 2016-04-27 12:58 ` Hans Verkuil 2016-04-27 13:14 ` Marek Szyprowski 0 siblings, 1 reply; 8+ messages in thread From: Hans Verkuil @ 2016-04-27 12:58 UTC (permalink / raw) To: Marek Szyprowski, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size 2016-04-27 12:58 ` Hans Verkuil @ 2016-04-27 13:14 ` Marek Szyprowski 2016-04-27 13:33 ` Hans Verkuil 0 siblings, 1 reply; 8+ messages in thread From: Marek Szyprowski @ 2016-04-27 13:14 UTC (permalink / raw) To: Hans Verkuil, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz Hello, On 2016-04-27 14:58, Hans Verkuil wrote: > 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? Nope, nothing breaks in such case. When no iommu is available this parameter is ignored by dma-mapping layer. Due to some other issues, it cannot be set by generic platform init code: http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size 2016-04-27 13:14 ` Marek Szyprowski @ 2016-04-27 13:33 ` Hans Verkuil 0 siblings, 0 replies; 8+ messages in thread From: Hans Verkuil @ 2016-04-27 13:33 UTC (permalink / raw) To: Marek Szyprowski, linux-media, linux-samsung-soc Cc: Sylwester Nawrocki, Kamil Debski, Laurent Pinchart, Krzysztof Kozlowski, Bartlomiej Zolnierkiewicz On 04/27/2016 03:14 PM, Marek Szyprowski wrote: > Hello, > > > On 2016-04-27 14:58, Hans Verkuil wrote: >> 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? > > Nope, nothing breaks in such case. When no iommu is available this parameter is > ignored by dma-mapping layer. Due to some other issues, it cannot be set by > generic platform init code: > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305913.html OK. Is there any reason this cannot be done in vb2-dma-contig.c itself? If there is no iommu, then this does nothing, and if there is one, don't you always need it? Requiring drivers to set this fairly obscure setting is asking for problems. This is guaranteed to be forgotten. As Lars-Peter said, ideally this is done by the DMA provider, but apparently that is (for now at least) a no-go. But the next best thing is vb2 IMHO. I gather that this is specific to userptr mode, so placing this code in vb2_dc_get_userptr() would make sense. You can even check the size here and do nothing if it is < 64 kB. Note sure if that is useful, though. Don't put it in vb2_dma_contig_init_ctx though as I am trying to get rid of that: http://www.mail-archive.com/linux-media@vger.kernel.org/msg96741.html Regards, Hans ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-04-27 13:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-27 12:00 [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Marek Szyprowski 2016-04-27 12:00 ` [PATCH RESEND 2/2] media: set proper max seg size for devices on Exynos SoCs Marek Szyprowski 2016-04-27 12:08 ` Hans Verkuil 2016-04-27 12:10 ` [PATCH RESEND 1/2] media: vb2-dma-contig: add helper for setting dma max seg size Hans Verkuil 2016-04-27 12:23 ` Marek Szyprowski 2016-04-27 12:58 ` Hans Verkuil 2016-04-27 13:14 ` Marek Szyprowski 2016-04-27 13:33 ` Hans Verkuil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox