From 29debab0a94035a390801d1f177d171d014b7765 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 3 May 2016 16:27:16 -0400 Subject: [PATCH 1/9] s5p-mfc: Set device name for reserved memory region devs The devices don't have a name set, so makes dev_name() returns NULL which makes harder to identify the devices that are causing issues, for example: WARNING: CPU: 2 PID: 616 at drivers/base/core.c:251 device_release+0x8c/0x90 Device '(null)' does not have a release() function, it is broken and must be fixed. And after setting the device name: WARNING: CPU: 0 PID: 591 at drivers/base/core.c:251 device_release+0x8c/0x90 Device 's5p-mfc-l' does not have a release() function, it is broken and must be fixed. Cc: Fixes: 6e83e6e25eb4 ("[media] s5p-mfc: Fix kernel warning on memory init") Signed-off-by: Javier Martinez Canillas Tested-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index b16466fe35ee..8fc1fd4ee2ed 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -1062,6 +1062,8 @@ static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) mfc_err("Not enough memory\n"); return -ENOMEM; } + + dev_set_name(dev->mem_dev_l, "%s", "s5p-mfc-l"); device_initialize(dev->mem_dev_l); of_property_read_u32_array(dev->plat_dev->dev.of_node, "samsung,mfc-l", mem_info, 2); @@ -1079,6 +1081,8 @@ static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) mfc_err("Not enough memory\n"); return -ENOMEM; } + + dev_set_name(dev->mem_dev_r, "%s", "s5p-mfc-r"); device_initialize(dev->mem_dev_r); of_property_read_u32_array(dev->plat_dev->dev.of_node, "samsung,mfc-r", mem_info, 2); From 6311f1261f59ce5e51fbe5cc3b5e7737197316ac Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 3 May 2016 16:27:17 -0400 Subject: [PATCH 2/9] s5p-mfc: Add release callback for memory region devs When s5p_mfc_remove() calls put_device() for the reserved memory region devs, the driver core warns that the dev doesn't have a release callback: WARNING: CPU: 0 PID: 591 at drivers/base/core.c:251 device_release+0x8c/0x90 Device 's5p-mfc-l' does not have a release() function, it is broken and must be fixed. Also, the declared DMA memory using dma_declare_coherent_memory() isn't relased so add a dev .release that calls dma_release_declared_memory(). Cc: Fixes: 6e83e6e25eb4 ("[media] s5p-mfc: Fix kernel warning on memory init") Signed-off-by: Javier Martinez Canillas Tested-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index 8fc1fd4ee2ed..beb4fd5bd326 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -1050,6 +1050,11 @@ static int match_child(struct device *dev, void *data) return !strcmp(dev_name(dev), (char *)data); } +static void s5p_mfc_memdev_release(struct device *dev) +{ + dma_release_declared_memory(dev); +} + static void *mfc_get_drv_data(struct platform_device *pdev); static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) @@ -1064,6 +1069,7 @@ static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) } dev_set_name(dev->mem_dev_l, "%s", "s5p-mfc-l"); + dev->mem_dev_l->release = s5p_mfc_memdev_release; device_initialize(dev->mem_dev_l); of_property_read_u32_array(dev->plat_dev->dev.of_node, "samsung,mfc-l", mem_info, 2); @@ -1083,6 +1089,7 @@ static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) } dev_set_name(dev->mem_dev_r, "%s", "s5p-mfc-r"); + dev->mem_dev_r->release = s5p_mfc_memdev_release; device_initialize(dev->mem_dev_r); of_property_read_u32_array(dev->plat_dev->dev.of_node, "samsung,mfc-r", mem_info, 2); From c974c436eaf406225b7f4c5d27ae033a12667a38 Mon Sep 17 00:00:00 2001 From: Javier Martinez Canillas Date: Tue, 3 May 2016 16:27:18 -0400 Subject: [PATCH 3/9] s5p-mfc: Fix race between s5p_mfc_probe() and s5p_mfc_open() The s5p_mfc_probe() function registers the video devices before all the resources needed by s5p_mfc_open() are correctly initalized. So if s5p_mfc_open() function is called before s5p_mfc_probe() finishes (since the video dev is already registered), a NULL pointer dereference will happen due s5p_mfc_open() accessing uninitialized vars such as the struct s5p_mfc_dev .watchdog_timer and .mfc_ops fields. An example is following BUG caused by add_timer() getting a NULL pointer: [ 45.765374] kernel BUG at kernel/time/timer.c:790! [ 45.765381] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM ... [ 45.766149] [] (mod_timer) from [] (s5p_mfc_open+0x274/0x4d4 [s5p_mfc]) [ 45.766416] [] (s5p_mfc_open [s5p_mfc]) from [] (v4l2_open+0x9c/0x100 [videodev]) [ 45.766547] [] (v4l2_open [videodev]) from [] (chrdev_open+0x9c/0x178) [ 45.766575] [] (chrdev_open) from [] (do_dentry_open+0x1e0/0x300) [ 45.766595] [] (do_dentry_open) from [] (path_openat+0x800/0x10d4) [ 45.766610] [] (path_openat) from [] (do_filp_open+0x5c/0xc0) [ 45.766624] [] (do_filp_open) from [] (do_sys_open+0x10c/0x1bc) [ 45.766642] [] (do_sys_open) from [] (ret_fast_syscall+0x0/0x3c) [ 45.766655] Code: eaffffe3 e3a00001 e28dd008 e8bd81f0 (e7f001f2) Fix it by registering the video devs as the last step in s5p_mfc_probe(). Signed-off-by: Javier Martinez Canillas Tested-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 39 +++++++++++++----------- 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index beb4fd5bd326..501d822cad6b 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -1212,14 +1212,6 @@ static int s5p_mfc_probe(struct platform_device *pdev) vfd->vfl_dir = VFL_DIR_M2M; snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_DEC_NAME); dev->vfd_dec = vfd; - ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); - if (ret) { - v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); - video_device_release(vfd); - goto err_dec_reg; - } - v4l2_info(&dev->v4l2_dev, - "decoder registered as /dev/video%d\n", vfd->num); video_set_drvdata(vfd, dev); /* encoder */ @@ -1237,14 +1229,6 @@ static int s5p_mfc_probe(struct platform_device *pdev) vfd->vfl_dir = VFL_DIR_M2M; snprintf(vfd->name, sizeof(vfd->name), "%s", S5P_MFC_ENC_NAME); dev->vfd_enc = vfd; - ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0); - if (ret) { - v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); - video_device_release(vfd); - goto err_enc_reg; - } - v4l2_info(&dev->v4l2_dev, - "encoder registered as /dev/video%d\n", vfd->num); video_set_drvdata(vfd, dev); platform_set_drvdata(pdev, dev); @@ -1261,15 +1245,34 @@ static int s5p_mfc_probe(struct platform_device *pdev) s5p_mfc_init_hw_cmds(dev); s5p_mfc_init_regs(dev); + /* Register decoder and encoder */ + ret = video_register_device(dev->vfd_dec, VFL_TYPE_GRABBER, 0); + if (ret) { + v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); + video_device_release(dev->vfd_dec); + goto err_dec_reg; + } + v4l2_info(&dev->v4l2_dev, + "decoder registered as /dev/video%d\n", dev->vfd_dec->num); + + ret = video_register_device(dev->vfd_enc, VFL_TYPE_GRABBER, 0); + if (ret) { + v4l2_err(&dev->v4l2_dev, "Failed to register video device\n"); + video_device_release(dev->vfd_enc); + goto err_enc_reg; + } + v4l2_info(&dev->v4l2_dev, + "encoder registered as /dev/video%d\n", dev->vfd_enc->num); + pr_debug("%s--\n", __func__); return 0; /* Deinit MFC if probe had failed */ err_enc_reg: - video_device_release(dev->vfd_enc); -err_enc_alloc: video_unregister_device(dev->vfd_dec); err_dec_reg: + video_device_release(dev->vfd_enc); +err_enc_alloc: video_device_release(dev->vfd_dec); err_dec_alloc: v4l2_device_unregister(&dev->v4l2_dev); From 3f03396918962b2f8b888d02b23cd1e0c88bf5e5 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 09:16:06 +0200 Subject: [PATCH 4/9] media: vb2-dma-contig: add helper for setting dma max seg size 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. Till now, the only case when vb2-dma-contig really 'worked' was a case where userspace provided USERPTR buffer, which was in fact mmaped contiguous buffer from the other v4l2/drm device. Also DMABUF made of contiguous buffer worked only when its exporter did not split it into several chunks in the scatter-list. Any other buffer failed, regardless of the arch/platform used and the presence of the IOMMU of the device bus. This patch provides interface to fix this issue. Signed-off-by: Marek Szyprowski Signed-off-by: Sylwester Nawrocki --- .../media/v4l2-core/videobuf2-dma-contig.c | 53 +++++++++++++++++++ include/media/videobuf2-dma-contig.h | 2 + 2 files changed, 55 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 5361197f3e57..e3e47ace7daf 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -753,6 +753,59 @@ void vb2_dma_contig_cleanup_ctx(void *alloc_ctx) } EXPORT_SYMBOL_GPL(vb2_dma_contig_cleanup_ctx); +/** + * vb2_dma_contig_set_max_seg_size() - configure DMA max segment size + * @dev: device for configuring DMA parameters + * @size: size of DMA max segment size to set + * + * 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 sets 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. + * This function should be called from the drivers, which are known to + * operate on platforms with IOMMU and provide access to shared buffers + * (either USERPTR or DMABUF). This should be done before initializing + * videobuf2 queue. + */ +int vb2_dma_contig_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; +} +EXPORT_SYMBOL_GPL(vb2_dma_contig_set_max_seg_size); + +/* + * vb2_dma_contig_clear_max_seg_size() - release resources for DMA parameters + * @dev: device for configuring DMA parameters + * + * This function releases resources allocated to configure DMA parameters + * (see vb2_dma_contig_set_max_seg_size() function). It should be called from + * device drivers on driver remove. + */ +void vb2_dma_contig_clear_max_seg_size(struct device *dev) +{ + kfree(dev->dma_parms); + dev->dma_parms = NULL; +} +EXPORT_SYMBOL_GPL(vb2_dma_contig_clear_max_seg_size); + MODULE_DESCRIPTION("DMA-contig memory handling routines for videobuf2"); MODULE_AUTHOR("Pawel Osciak "); MODULE_LICENSE("GPL"); diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h index 2087c9a68be3..f7dc8401817e 100644 --- a/include/media/videobuf2-dma-contig.h +++ b/include/media/videobuf2-dma-contig.h @@ -35,6 +35,8 @@ 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); +void vb2_dma_contig_clear_max_seg_size(struct device *dev); extern const struct vb2_mem_ops vb2_dma_contig_memops; From 712b617e5e1a9084917c5f6ab1ccb1456b722f51 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 09:16:07 +0200 Subject: [PATCH 5/9] media: set proper max seg size for devices on Exynos SoCs 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 Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/exynos-gsc/gsc-core.c | 2 ++ drivers/media/platform/exynos4-is/fimc-core.c | 2 ++ drivers/media/platform/exynos4-is/fimc-is.c | 2 ++ drivers/media/platform/exynos4-is/fimc-lite.c | 2 ++ drivers/media/platform/s5p-g2d/g2d.c | 2 ++ drivers/media/platform/s5p-jpeg/jpeg-core.c | 2 ++ drivers/media/platform/s5p-mfc/s5p_mfc.c | 4 ++++ drivers/media/platform/s5p-tv/mixer_video.c | 2 ++ 8 files changed, 18 insertions(+) diff --git a/drivers/media/platform/exynos-gsc/gsc-core.c b/drivers/media/platform/exynos-gsc/gsc-core.c index c04973669a47..c9d2009c2476 100644 --- a/drivers/media/platform/exynos-gsc/gsc-core.c +++ b/drivers/media/platform/exynos-gsc/gsc-core.c @@ -1124,6 +1124,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); @@ -1153,6 +1154,7 @@ static int gsc_remove(struct platform_device *pdev) v4l2_device_unregister(&gsc->v4l2_dev); vb2_dma_contig_cleanup_ctx(gsc->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(&pdev->dev); pm_runtime_disable(&pdev->dev); gsc_clk_put(gsc); diff --git a/drivers/media/platform/exynos4-is/fimc-core.c b/drivers/media/platform/exynos4-is/fimc-core.c index b1c1cea82a27..368f44f24d4c 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); @@ -1124,6 +1125,7 @@ static int fimc_remove(struct platform_device *pdev) fimc_unregister_capture_subdev(fimc); vb2_dma_contig_cleanup_ctx(fimc->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(&pdev->dev); clk_disable(fimc->clock[CLK_BUS]); fimc_clk_put(fimc); diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c index 979c388ebf60..bd98b56318b7 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); @@ -940,6 +941,7 @@ static int fimc_is_remove(struct platform_device *pdev) free_irq(is->irq, is); fimc_is_unregister_subdevs(is); vb2_dma_contig_cleanup_ctx(is->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(dev); fimc_is_put_clocks(is); fimc_is_debugfs_remove(is); release_firmware(is->fw.f_w); diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c b/drivers/media/platform/exynos4-is/fimc-lite.c index dc1b929f7a33..27cb620cb714 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); @@ -1652,6 +1653,7 @@ static int fimc_lite_remove(struct platform_device *pdev) pm_runtime_set_suspended(dev); fimc_lite_unregister_capture_subdev(fimc); vb2_dma_contig_cleanup_ctx(fimc->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(dev); fimc_lite_clk_put(fimc); dev_info(dev, "Driver unloaded\n"); diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c index 612d1ea514f1..d3e3469db8de 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); @@ -757,6 +758,7 @@ static int g2d_remove(struct platform_device *pdev) video_unregister_device(dev->vfd); v4l2_device_unregister(&dev->v4l2_dev); vb2_dma_contig_cleanup_ctx(dev->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(&pdev->dev); clk_unprepare(dev->gate); clk_put(dev->gate); clk_unprepare(dev->clk); diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index caa19b408551..17bc94092864 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -2843,6 +2843,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"); @@ -2942,6 +2943,7 @@ static int s5p_jpeg_remove(struct platform_device *pdev) video_unregister_device(jpeg->vfd_decoder); video_unregister_device(jpeg->vfd_encoder); vb2_dma_contig_cleanup_ctx(jpeg->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(&pdev->dev); v4l2_m2m_release(jpeg->m2m_dev); v4l2_device_unregister(&jpeg->v4l2_dev); diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index 501d822cad6b..d1d9d38864d7 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -1175,11 +1175,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]); @@ -1307,6 +1309,8 @@ static int s5p_mfc_remove(struct platform_device *pdev) s5p_mfc_release_firmware(dev); vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]); vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]); + vb2_dma_contig_clear_max_seg_size(dev->mem_dev_l); + vb2_dma_contig_clear_max_seg_size(dev->mem_dev_r); if (pdev->dev.of_node) { put_device(dev->mem_dev_l); put_device(dev->mem_dev_r); diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c index 7ab5578a0405..123d27107f60 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"); @@ -152,6 +153,7 @@ void mxr_release_video(struct mxr_device *mdev) kfree(mdev->output[i]); vb2_dma_contig_cleanup_ctx(mdev->alloc_ctx); + vb2_dma_contig_clear_max_seg_size(mdev->dev); v4l2_device_unregister(&mdev->v4l2_dev); } From 59ce4039727ef408769ab0e1263ece893abe22bb Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 15:31:24 +0200 Subject: [PATCH 6/9] of: reserved_mem: add support for using more than one region for given device This patch allows device drivers to initialize more than one reserved memory region assigned to given device. When driver needs to use more than one reserved memory region, it should allocate child devices and initialize regions by index for each of its child devices. Signed-off-by: Marek Szyprowski Acked-by: Rob Herring Signed-off-by: Sylwester Nawrocki --- drivers/of/of_reserved_mem.c | 91 ++++++++++++++++++++++++--------- include/linux/of_reserved_mem.h | 25 +++++++-- 2 files changed, 89 insertions(+), 27 deletions(-) diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c index ed01c0172e4a..04e4fe58fb0c 100644 --- a/drivers/of/of_reserved_mem.c +++ b/drivers/of/of_reserved_mem.c @@ -21,6 +21,7 @@ #include #include #include +#include #define MAX_RESERVED_REGIONS 16 static struct reserved_mem reserved_mem[MAX_RESERVED_REGIONS]; @@ -289,53 +290,95 @@ static inline struct reserved_mem *__find_rmem(struct device_node *node) return NULL; } -/** - * of_reserved_mem_device_init() - assign reserved memory region to given device - * - * This function assign memory region pointed by "memory-region" device tree - * property to the given device. - */ -int of_reserved_mem_device_init(struct device *dev) -{ +struct rmem_assigned_device { + struct device *dev; + struct reserved_mem *rmem; + struct list_head list; +}; + +static LIST_HEAD(of_rmem_assigned_device_list); +static DEFINE_MUTEX(of_rmem_assigned_device_mutex); + +/** + * of_reserved_mem_device_init_by_idx() - assign reserved memory region to + * given device + * @dev: Pointer to the device to configure + * @np: Pointer to the device_node with 'reserved-memory' property + * @idx: Index of selected region + * + * This function assigns respective DMA-mapping operations based on reserved + * memory region specified by 'memory-region' property in @np node to the @dev + * device. When driver needs to use more than one reserved memory region, it + * should allocate child devices and initialize regions by name for each of + * child device. + * + * Returns error code or zero on success. + */ +int of_reserved_mem_device_init_by_idx(struct device *dev, + struct device_node *np, int idx) +{ + struct rmem_assigned_device *rd; + struct device_node *target; struct reserved_mem *rmem; - struct device_node *np; int ret; - np = of_parse_phandle(dev->of_node, "memory-region", 0); - if (!np) - return -ENODEV; + if (!np || !dev) + return -EINVAL; - rmem = __find_rmem(np); - of_node_put(np); + target = of_parse_phandle(np, "memory-region", idx); + if (!target) + return -EINVAL; + + rmem = __find_rmem(target); + of_node_put(target); if (!rmem || !rmem->ops || !rmem->ops->device_init) return -EINVAL; + rd = kmalloc(sizeof(struct rmem_assigned_device), GFP_KERNEL); + if (!rd) + return -ENOMEM; + ret = rmem->ops->device_init(rmem, dev); - if (ret == 0) + if (ret == 0) { + rd->dev = dev; + rd->rmem = rmem; + + mutex_lock(&of_rmem_assigned_device_mutex); + list_add(&rd->list, &of_rmem_assigned_device_list); + mutex_unlock(&of_rmem_assigned_device_mutex); + dev_info(dev, "assigned reserved memory node %s\n", rmem->name); + } else { + kfree(rd); + } return ret; } -EXPORT_SYMBOL_GPL(of_reserved_mem_device_init); +EXPORT_SYMBOL_GPL(of_reserved_mem_device_init_by_idx); /** * of_reserved_mem_device_release() - release reserved memory device structures + * @dev: Pointer to the device to deconfigure * * This function releases structures allocated for memory region handling for * the given device. */ void of_reserved_mem_device_release(struct device *dev) { - struct reserved_mem *rmem; - struct device_node *np; + struct rmem_assigned_device *rd; + struct reserved_mem *rmem = NULL; - np = of_parse_phandle(dev->of_node, "memory-region", 0); - if (!np) - return; - - rmem = __find_rmem(np); - of_node_put(np); + mutex_lock(&of_rmem_assigned_device_mutex); + list_for_each_entry(rd, &of_rmem_assigned_device_list, list) { + if (rd->dev == dev) { + rmem = rd->rmem; + list_del(&rd->list); + kfree(rd); + break; + } + } + mutex_unlock(&of_rmem_assigned_device_mutex); if (!rmem || !rmem->ops || !rmem->ops->device_release) return; diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h index ad2f67054372..1779cda99ef7 100644 --- a/include/linux/of_reserved_mem.h +++ b/include/linux/of_reserved_mem.h @@ -1,7 +1,8 @@ #ifndef __OF_RESERVED_MEM_H #define __OF_RESERVED_MEM_H -struct device; +#include + struct of_phandle_args; struct reserved_mem_ops; @@ -28,14 +29,17 @@ typedef int (*reservedmem_of_init_fn)(struct reserved_mem *rmem); _OF_DECLARE(reservedmem, name, compat, init, reservedmem_of_init_fn) #ifdef CONFIG_OF_RESERVED_MEM -int of_reserved_mem_device_init(struct device *dev); + +int of_reserved_mem_device_init_by_idx(struct device *dev, + struct device_node *np, int idx); void of_reserved_mem_device_release(struct device *dev); void fdt_init_reserved_mem(void); void fdt_reserved_mem_save_node(unsigned long node, const char *uname, phys_addr_t base, phys_addr_t size); #else -static inline int of_reserved_mem_device_init(struct device *dev) +static inline int of_reserved_mem_device_init_by_idx(struct device *dev, + struct device_node *np, int idx) { return -ENOSYS; } @@ -46,4 +50,19 @@ static inline void fdt_reserved_mem_save_node(unsigned long node, const char *uname, phys_addr_t base, phys_addr_t size) { } #endif +/** + * of_reserved_mem_device_init() - assign reserved memory region to given device + * @dev: Pointer to the device to configure + * + * This function assigns respective DMA-mapping operations based on the first + * reserved memory region specified by 'memory-region' property in device tree + * node of the given device. + * + * Returns error code or zero on success. + */ +static inline int of_reserved_mem_device_init(struct device *dev) +{ + return of_reserved_mem_device_init_by_idx(dev, dev->of_node, 0); +} + #endif /* __OF_RESERVED_MEM_H */ From 6a8d73e2859df06540d37877b1c1a0c9b5d11292 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 15:31:25 +0200 Subject: [PATCH 7/9] media: s5p-mfc: use generic reserved memory bindings Use generic reserved memory bindings and mark old, custom properties as obsoleted. Signed-off-by: Marek Szyprowski Reviewed-by: Javier Martinez Canillas Acked-by: Rob Herring Acked-by: Krzysztof Kozlowski Signed-off-by: Sylwester Nawrocki --- .../devicetree/bindings/media/s5p-mfc.txt | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt index 2d5787eac91a..92c94f5ecbf1 100644 --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt @@ -21,15 +21,18 @@ Required properties: - clock-names : from common clock binding: must contain "mfc", corresponding to entry in the clocks property. - - samsung,mfc-r : Base address of the first memory bank used by MFC - for DMA contiguous memory allocation and its size. - - - samsung,mfc-l : Base address of the second memory bank used by MFC - for DMA contiguous memory allocation and its size. - Optional properties: - power-domains : power-domain property defined with a phandle to respective power domain. + - memory-region : from reserved memory binding: phandles to two reserved + memory regions, first is for "left" mfc memory bus interfaces, + second if for the "right" mfc memory bus, used when no SYSMMU + support is available + +Obsolete properties: + - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region + property instead + Example: SoC specific DT entry: @@ -43,9 +46,29 @@ mfc: codec@13400000 { clock-names = "mfc"; }; +Reserved memory specific DT entry for given board (see reserved memory binding +for more information): + +reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + mfc_left: region@51000000 { + compatible = "shared-dma-pool"; + no-map; + reg = <0x51000000 0x800000>; + }; + + mfc_right: region@43000000 { + compatible = "shared-dma-pool"; + no-map; + reg = <0x43000000 0x800000>; + }; +}; + Board specific DT entry: codec@13400000 { - samsung,mfc-r = <0x43000000 0x800000>; - samsung,mfc-l = <0x51000000 0x800000>; + memory-region = <&mfc_left>, <&mfc_right>; }; From c79667dd93b084fe412bcfe7fbf0ba43f7dec520 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 15:31:26 +0200 Subject: [PATCH 8/9] media: s5p-mfc: replace custom reserved memory handling code with generic one This patch removes custom code for initialization and handling of reserved memory regions in s5p-mfc driver and replaces it with generic reserved memory regions api. s5p-mfc driver now handles two reserved memory regions defined by generic reserved memory bindings. Support for non-dt platform has been removed, because all supported platforms have been already converted to device tree. Signed-off-by: Marek Szyprowski Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 154 +++++++++++------------ 1 file changed, 71 insertions(+), 83 deletions(-) diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index d1d9d38864d7..fff5f4369802 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include "s5p_mfc_common.h" #include "s5p_mfc_ctrl.h" @@ -1043,66 +1044,71 @@ static const struct v4l2_file_operations s5p_mfc_fops = { .mmap = s5p_mfc_mmap, }; -static int match_child(struct device *dev, void *data) -{ - if (!dev_name(dev)) - return 0; - return !strcmp(dev_name(dev), (char *)data); -} - +/* DMA memory related helper functions */ static void s5p_mfc_memdev_release(struct device *dev) { - dma_release_declared_memory(dev); + of_reserved_mem_device_release(dev); +} + +static struct device *s5p_mfc_alloc_memdev(struct device *dev, + const char *name, unsigned int idx) +{ + struct device *child; + int ret; + + child = devm_kzalloc(dev, sizeof(struct device), GFP_KERNEL); + if (!child) + return NULL; + + device_initialize(child); + dev_set_name(child, "%s:%s", dev_name(dev), name); + child->parent = dev; + child->bus = dev->bus; + child->coherent_dma_mask = dev->coherent_dma_mask; + child->dma_mask = dev->dma_mask; + child->release = s5p_mfc_memdev_release; + + if (device_add(child) == 0) { + ret = of_reserved_mem_device_init_by_idx(child, dev->of_node, + idx); + if (ret == 0) + return child; + } + + put_device(child); + return NULL; +} + +static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev) +{ + struct device *dev = &mfc_dev->plat_dev->dev; + + /* + * Create and initialize virtual devices for accessing + * reserved memory regions. + */ + mfc_dev->mem_dev_l = s5p_mfc_alloc_memdev(dev, "left", + MFC_BANK1_ALLOC_CTX); + if (!mfc_dev->mem_dev_l) + return -ENODEV; + mfc_dev->mem_dev_r = s5p_mfc_alloc_memdev(dev, "right", + MFC_BANK2_ALLOC_CTX); + if (!mfc_dev->mem_dev_r) { + device_unregister(mfc_dev->mem_dev_l); + return -ENODEV; + } + + return 0; +} + +static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev) +{ + device_unregister(mfc_dev->mem_dev_l); + device_unregister(mfc_dev->mem_dev_r); } static void *mfc_get_drv_data(struct platform_device *pdev); -static int s5p_mfc_alloc_memdevs(struct s5p_mfc_dev *dev) -{ - unsigned int mem_info[2] = { }; - - dev->mem_dev_l = devm_kzalloc(&dev->plat_dev->dev, - sizeof(struct device), GFP_KERNEL); - if (!dev->mem_dev_l) { - mfc_err("Not enough memory\n"); - return -ENOMEM; - } - - dev_set_name(dev->mem_dev_l, "%s", "s5p-mfc-l"); - dev->mem_dev_l->release = s5p_mfc_memdev_release; - device_initialize(dev->mem_dev_l); - of_property_read_u32_array(dev->plat_dev->dev.of_node, - "samsung,mfc-l", mem_info, 2); - if (dma_declare_coherent_memory(dev->mem_dev_l, mem_info[0], - mem_info[0], mem_info[1], - DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) { - mfc_err("Failed to declare coherent memory for\n" - "MFC device\n"); - return -ENOMEM; - } - - dev->mem_dev_r = devm_kzalloc(&dev->plat_dev->dev, - sizeof(struct device), GFP_KERNEL); - if (!dev->mem_dev_r) { - mfc_err("Not enough memory\n"); - return -ENOMEM; - } - - dev_set_name(dev->mem_dev_r, "%s", "s5p-mfc-r"); - dev->mem_dev_r->release = s5p_mfc_memdev_release; - device_initialize(dev->mem_dev_r); - of_property_read_u32_array(dev->plat_dev->dev.of_node, - "samsung,mfc-r", mem_info, 2); - if (dma_declare_coherent_memory(dev->mem_dev_r, mem_info[0], - mem_info[0], mem_info[1], - DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE) == 0) { - pr_err("Failed to declare coherent memory for\n" - "MFC device\n"); - return -ENOMEM; - } - return 0; -} - /* MFC probe function */ static int s5p_mfc_probe(struct platform_device *pdev) { @@ -1128,12 +1134,6 @@ static int s5p_mfc_probe(struct platform_device *pdev) dev->variant = mfc_get_drv_data(pdev); - ret = s5p_mfc_init_pm(dev); - if (ret < 0) { - dev_err(&pdev->dev, "failed to get mfc clock source\n"); - return ret; - } - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); dev->regs_base = devm_ioremap_resource(&pdev->dev, res); @@ -1154,25 +1154,16 @@ static int s5p_mfc_probe(struct platform_device *pdev) goto err_res; } - if (pdev->dev.of_node) { - ret = s5p_mfc_alloc_memdevs(dev); - if (ret < 0) - goto err_res; - } else { - dev->mem_dev_l = device_find_child(&dev->plat_dev->dev, - "s5p-mfc-l", match_child); - if (!dev->mem_dev_l) { - mfc_err("Mem child (L) device get failed\n"); - ret = -ENODEV; - goto err_res; - } - dev->mem_dev_r = device_find_child(&dev->plat_dev->dev, - "s5p-mfc-r", match_child); - if (!dev->mem_dev_r) { - mfc_err("Mem child (R) device get failed\n"); - ret = -ENODEV; - goto err_res; - } + ret = s5p_mfc_configure_dma_memory(dev); + if (ret < 0) { + dev_err(&pdev->dev, "failed to configure DMA memory\n"); + return ret; + } + + ret = s5p_mfc_init_pm(dev); + if (ret < 0) { + dev_err(&pdev->dev, "failed to get mfc clock source\n"); + return ret; } vb2_dma_contig_set_max_seg_size(dev->mem_dev_l, DMA_BIT_MASK(32)); @@ -1309,12 +1300,9 @@ static int s5p_mfc_remove(struct platform_device *pdev) s5p_mfc_release_firmware(dev); vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[0]); vb2_dma_contig_cleanup_ctx(dev->alloc_ctx[1]); + s5p_mfc_unconfigure_dma_memory(dev); vb2_dma_contig_clear_max_seg_size(dev->mem_dev_l); vb2_dma_contig_clear_max_seg_size(dev->mem_dev_r); - if (pdev->dev.of_node) { - put_device(dev->mem_dev_l); - put_device(dev->mem_dev_r); - } s5p_mfc_final_pm(dev); return 0; From 04f776734c4e03e33111d3d5a994b589870df623 Mon Sep 17 00:00:00 2001 From: Marek Szyprowski Date: Tue, 24 May 2016 15:31:27 +0200 Subject: [PATCH 9/9] media: s5p-mfc: add iommu support This patch adds support for IOMMU to s5p-mfc device driver. MFC firmware is limited and it cannot use the default configuration. If IOMMU is available, the patch disables the default DMA address space configuration and creates a new address space of size limited to 256M and base address set to 0x20000000. For now the same address space is shared by both 'left' and 'right' memory channels, because the DMA/IOMMU frameworks do not support configuring them separately. This is not optimal, but besides limiting total address space available has no other drawbacks (MFC firmware supports 256M of address space per each channel). Signed-off-by: Marek Szyprowski Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas Signed-off-by: Sylwester Nawrocki --- drivers/media/platform/s5p-mfc/s5p_mfc.c | 24 ++++++ .../media/platform/s5p-mfc/s5p_mfc_iommu.h | 79 +++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c index fff5f4369802..6ee620ee8cd5 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c @@ -30,6 +30,7 @@ #include "s5p_mfc_dec.h" #include "s5p_mfc_enc.h" #include "s5p_mfc_intr.h" +#include "s5p_mfc_iommu.h" #include "s5p_mfc_opr.h" #include "s5p_mfc_cmd.h" #include "s5p_mfc_pm.h" @@ -1083,6 +1084,22 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev) { struct device *dev = &mfc_dev->plat_dev->dev; + /* + * When IOMMU is available, we cannot use the default configuration, + * because of MFC firmware requirements: address space limited to + * 256M and non-zero default start address. + * This is still simplified, not optimal configuration, but for now + * IOMMU core doesn't allow to configure device's IOMMUs channel + * separately. + */ + if (exynos_is_iommu_available(dev)) { + int ret = exynos_configure_iommu(dev, S5P_MFC_IOMMU_DMA_BASE, + S5P_MFC_IOMMU_DMA_SIZE); + if (ret == 0) + mfc_dev->mem_dev_l = mfc_dev->mem_dev_r = dev; + return ret; + } + /* * Create and initialize virtual devices for accessing * reserved memory regions. @@ -1103,6 +1120,13 @@ static int s5p_mfc_configure_dma_memory(struct s5p_mfc_dev *mfc_dev) static void s5p_mfc_unconfigure_dma_memory(struct s5p_mfc_dev *mfc_dev) { + struct device *dev = &mfc_dev->plat_dev->dev; + + if (exynos_is_iommu_available(dev)) { + exynos_unconfigure_iommu(dev); + return; + } + device_unregister(mfc_dev->mem_dev_l); device_unregister(mfc_dev->mem_dev_r); } diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h new file mode 100644 index 000000000000..5d1d1c2922e8 --- /dev/null +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_iommu.h @@ -0,0 +1,79 @@ +/* + * Copyright (C) 2015 Samsung Electronics Co.Ltd + * Authors: Marek Szyprowski + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#ifndef S5P_MFC_IOMMU_H_ +#define S5P_MFC_IOMMU_H_ + +#define S5P_MFC_IOMMU_DMA_BASE 0x20000000lu +#define S5P_MFC_IOMMU_DMA_SIZE SZ_256M + +#ifdef CONFIG_EXYNOS_IOMMU + +#include + +static inline bool exynos_is_iommu_available(struct device *dev) +{ + return dev->archdata.iommu != NULL; +} + +static inline void exynos_unconfigure_iommu(struct device *dev) +{ + struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev); + + arm_iommu_detach_device(dev); + arm_iommu_release_mapping(mapping); +} + +static inline int exynos_configure_iommu(struct device *dev, + unsigned int base, unsigned int size) +{ + struct dma_iommu_mapping *mapping = NULL; + int ret; + + /* Disable the default mapping created by device core */ + if (to_dma_iommu_mapping(dev)) + exynos_unconfigure_iommu(dev); + + mapping = arm_iommu_create_mapping(dev->bus, base, size); + if (IS_ERR(mapping)) { + pr_warn("Failed to create IOMMU mapping for device %s\n", + dev_name(dev)); + return PTR_ERR(mapping); + } + + ret = arm_iommu_attach_device(dev, mapping); + if (ret) { + pr_warn("Failed to attached device %s to IOMMU_mapping\n", + dev_name(dev)); + arm_iommu_release_mapping(mapping); + return ret; + } + + return 0; +} + +#else + +static inline bool exynos_is_iommu_available(struct device *dev) +{ + return false; +} + +static inline int exynos_configure_iommu(struct device *dev, + unsigned int base, unsigned int size) +{ + return -ENOSYS; +} + +static inline void exynos_unconfigure_iommu(struct device *dev) { } + +#endif + +#endif /* S5P_MFC_IOMMU_H_ */