[PATCH v1 0/4] NVIDIA Tegra Video Decoder driver improvements

[PATCH v1 0/4] NVIDIA Tegra Video Decoder driver improvements

[Dmitry Osipenko]

Hello,

This series cleans up some of minor shortcomings that are caused by
checkpatch recommendations that are not very applicable for the driver.
Then IOMMU support is added to the driver and now it can handle sparse
memory buffers that GPU hands to VDE in a default kernel configuration
on Tegra30+.

Dmitry Osipenko (4):
  staging: media: tegra-vde: Remove BIT() macro from UAPI header
  staging: media: tegra-vde: Manually pack UAPI structures
  staging: media: tegra-vde: Add IOMMU support
  staging: media: tegra-vde: Defer dmabuf's unmapping

 drivers/staging/media/tegra-vde/Kconfig       |   1 +
 drivers/staging/media/tegra-vde/Makefile      |   1 +
 .../staging/media/tegra-vde/dmabuf-cache.c    | 223 ++++++++++++++++++
 drivers/staging/media/tegra-vde/iommu.c       | 146 ++++++++++++
 drivers/staging/media/tegra-vde/trace.h       |   1 +
 drivers/staging/media/tegra-vde/uapi.h        |  48 ++--
 .../media/tegra-vde/{tegra-vde.c => vde.c}    | 212 ++++++-----------
 drivers/staging/media/tegra-vde/vde.h         | 105 +++++++++
 8 files changed, 574 insertions(+), 163 deletions(-)
 create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c
 create mode 100644 drivers/staging/media/tegra-vde/iommu.c
 rename drivers/staging/media/tegra-vde/{tegra-vde.c => vde.c} (88%)
 create mode 100644 drivers/staging/media/tegra-vde/vde.h

-- 
2.21.0

[PATCH v1] ARM: dts: tegra30: Connect SMMU with Video Decoder Engine

[Dmitry Osipenko]

Enable IOMMU support for the video decoder.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/boot/dts/tegra30.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 934caa83c8db..ce162125e7bf 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -424,6 +424,7 @@
 		clocks = <&tegra_car TEGRA30_CLK_VDE>;
 		reset-names = "vde", "mc";
 		resets = <&tegra_car 61>, <&mc TEGRA30_MC_RESET_VDE>;
+		iommus = <&mc TEGRA_SWGROUP_VDE>;
 	};
 
 	apbmisc@70000800 {
-- 
2.21.0

[PATCH v1] ARM: tegra: Enable Tegra VDE driver in tegra_defconfig

[Dmitry Osipenko]

The video decoder driver was tested by time and works absolutely fine.
The reason why it is in staging is because it doesn't provide common V4L
interface yet, this shouldn't stop driver enabling in the defconfig since
our userspace (libvdpau-tegra) provides combined acceleration of decoding
and displaying without use of V4L.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 arch/arm/configs/tegra_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 8f5c6a5b444c..a27592d3b1fa 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -250,6 +250,8 @@ CONFIG_KEYBOARD_NVEC=y
 CONFIG_SERIO_NVEC_PS2=y
 CONFIG_NVEC_POWER=y
 CONFIG_NVEC_PAZ00=y
+CONFIG_STAGING_MEDIA=y
+CONFIG_TEGRA_VDE=y
 CONFIG_TEGRA_IOMMU_GART=y
 CONFIG_TEGRA_IOMMU_SMMU=y
 CONFIG_ARCH_TEGRA_2x_SOC=y
-- 
2.21.0

[PATCH v1] media: dt: bindings: tegra-vde: Document new optional IOMMU property

[Dmitry Osipenko]

All NVIDIA Tegra SoC generations provide IOMMU support for the video
decoder engine. Document new optional device-tree property that connects
VDE with the IOMMU provider.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
index 7302e949e662..602169b8aa19 100644
--- a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
+++ b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
@@ -35,6 +35,7 @@ Optional properties:
 - resets : Must contain an entry for each entry in reset-names.
 - reset-names : Must include the following entries:
   - mc
+- iommus: Must contain phandle to the IOMMU device node.
 
 Example:
 
@@ -59,4 +60,5 @@ video-codec@6001a000 {
 	clocks = <&tegra_car TEGRA20_CLK_VDE>;
 	reset-names = "vde", "mc";
 	resets = <&tegra_car 61>, <&mc TEGRA20_MC_RESET_VDE>;
+	iommus = <&mc TEGRA_SWGROUP_VDE>;
 };
-- 
2.21.0

[PATCH v1] media: dt: bindings: tegra-vde: Document new optional IOMMU property

[Dmitry Osipenko]

All NVIDIA Tegra SoC generations provide IOMMU support for the video
decoder engine. Document new optional device-tree property that connects
VDE with the IOMMU provider.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
index 7302e949e662..602169b8aa19 100644
--- a/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
+++ b/Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
@@ -35,6 +35,7 @@ Optional properties:
 - resets : Must contain an entry for each entry in reset-names.
 - reset-names : Must include the following entries:
   - mc
+- iommus: Must contain phandle to the IOMMU device node.
 
 Example:
 
@@ -59,4 +60,5 @@ video-codec@6001a000 {
 	clocks = <&tegra_car TEGRA20_CLK_VDE>;
 	reset-names = "vde", "mc";
 	resets = <&tegra_car 61>, <&mc TEGRA20_MC_RESET_VDE>;
+	iommus = <&mc TEGRA_SWGROUP_VDE>;
 };
-- 
2.21.0

[PATCH v1 1/4] staging: media: tegra-vde: Remove BIT() macro from UAPI header

[Dmitry Osipenko]

The BIT macro isn't available in userspace. Checkpatch complains about
shifts being used instead of the macro and people are starting to send
patches without realizing that it's a UAPI header file. Hence let's
replace the BIT macro with a hex values to make everyone happy.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/uapi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/uapi.h b/drivers/staging/media/tegra-vde/uapi.h
index a0dad1ed94ef..dd3e4a8c9f7e 100644
--- a/drivers/staging/media/tegra-vde/uapi.h
+++ b/drivers/staging/media/tegra-vde/uapi.h
@@ -6,8 +6,8 @@
 #include <linux/types.h>
 #include <asm/ioctl.h>
 
-#define FLAG_B_FRAME		BIT(0)
-#define FLAG_REFERENCE		BIT(1)
+#define FLAG_B_FRAME		0x1
+#define FLAG_REFERENCE		0x2
 
 struct tegra_vde_h264_frame {
 	__s32 y_fd;
-- 
2.21.0

[PATCH v1 1/4] staging: media: tegra-vde: Remove BIT() macro from UAPI header

[Dmitry Osipenko]

The BIT macro isn't available in userspace. Checkpatch complains about
shifts being used instead of the macro and people are starting to send
patches without realizing that it's a UAPI header file. Hence let's
replace the BIT macro with a hex values to make everyone happy.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/uapi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/uapi.h b/drivers/staging/media/tegra-vde/uapi.h
index a0dad1ed94ef..dd3e4a8c9f7e 100644
--- a/drivers/staging/media/tegra-vde/uapi.h
+++ b/drivers/staging/media/tegra-vde/uapi.h
@@ -6,8 +6,8 @@
 #include <linux/types.h>
 #include <asm/ioctl.h>
 
-#define FLAG_B_FRAME		BIT(0)
-#define FLAG_REFERENCE		BIT(1)
+#define FLAG_B_FRAME		0x1
+#define FLAG_REFERENCE		0x2
 
 struct tegra_vde_h264_frame {
 	__s32 y_fd;
-- 
2.21.0

[PATCH v1 2/4] staging: media: tegra-vde: Manually pack UAPI structures

[Dmitry Osipenko]

The __packed macro isn't available in userspace with the kernel headers.
Checkpatch asks to use the macro, which is unwanted in a case of a UAPI
header. There is no much benefit in a tight packing of the structures,
hence let's pack them manually to cleanup things a tad. Note that there
is no old-stable userspace that will suffer from this change, hence it's
fine to change the ABI. In a result also more space is reserved for a
possible future expansion of the UAPI as it was already shown that more
fields will be needed for a later SoC generations.

Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 13 +++++-
 drivers/staging/media/tegra-vde/uapi.h      | 44 +++++++++++----------
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/tegra-vde.c
index a5020dbf6eef..cc4244da2705 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -795,7 +795,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 {
 	struct device *dev = vde->miscdev.parent;
 	struct tegra_vde_h264_decoder_ctx ctx;
-	struct tegra_vde_h264_frame frames[17];
+	struct tegra_vde_h264_frame *frames;
 	struct tegra_vde_h264_frame __user *frames_user;
 	struct video_frame *dpb_frames;
 	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
@@ -830,11 +830,17 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	if (ret)
 		return ret;
 
+	frames = kmalloc_array(ctx.dpb_frames_nb, sizeof(*frames), GFP_KERNEL);
+	if (!frames) {
+		ret = -ENOMEM;
+		goto release_bitstream_dmabuf;
+	}
+
 	dpb_frames = kcalloc(ctx.dpb_frames_nb, sizeof(*dpb_frames),
 			     GFP_KERNEL);
 	if (!dpb_frames) {
 		ret = -ENOMEM;
-		goto release_bitstream_dmabuf;
+		goto free_frames;
 	}
 
 	macroblocks_nb = ctx.pic_width_in_mbs * ctx.pic_height_in_mbs;
@@ -955,6 +961,9 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 free_dpb_frames:
 	kfree(dpb_frames);
 
+free_frames:
+	kfree(frames);
+
 release_bitstream_dmabuf:
 	tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
 					bitstream_sgt, DMA_TO_DEVICE);
diff --git a/drivers/staging/media/tegra-vde/uapi.h b/drivers/staging/media/tegra-vde/uapi.h
index dd3e4a8c9f7e..ffb4983e5bb6 100644
--- a/drivers/staging/media/tegra-vde/uapi.h
+++ b/drivers/staging/media/tegra-vde/uapi.h
@@ -21,40 +21,42 @@ struct tegra_vde_h264_frame {
 	__u32 frame_num;
 	__u32 flags;
 
-	__u32 reserved;
-} __attribute__((packed));
+	// Must be zero'ed
+	__u32 reserved[6];
+};
 
 struct tegra_vde_h264_decoder_ctx {
 	__s32 bitstream_data_fd;
 	__u32 bitstream_data_offset;
 
 	__u64 dpb_frames_ptr;
-	__u8  dpb_frames_nb;
-	__u8  dpb_ref_frames_with_earlier_poc_nb;
+	__u32 dpb_frames_nb;
+	__u32 dpb_ref_frames_with_earlier_poc_nb;
 
 	// SPS
-	__u8  baseline_profile;
-	__u8  level_idc;
-	__u8  log2_max_pic_order_cnt_lsb;
-	__u8  log2_max_frame_num;
-	__u8  pic_order_cnt_type;
-	__u8  direct_8x8_inference_flag;
-	__u8  pic_width_in_mbs;
-	__u8  pic_height_in_mbs;
+	__u32 baseline_profile;
+	__u32 level_idc;
+	__u32 log2_max_pic_order_cnt_lsb;
+	__u32 log2_max_frame_num;
+	__u32 pic_order_cnt_type;
+	__u32 direct_8x8_inference_flag;
+	__u32 pic_width_in_mbs;
+	__u32 pic_height_in_mbs;
 
 	// PPS
-	__u8  pic_init_qp;
-	__u8  deblocking_filter_control_present_flag;
-	__u8  constrained_intra_pred_flag;
-	__u8  chroma_qp_index_offset;
-	__u8  pic_order_present_flag;
+	__u32 pic_init_qp;
+	__u32 deblocking_filter_control_present_flag;
+	__u32 constrained_intra_pred_flag;
+	__u32 chroma_qp_index_offset;
+	__u32 pic_order_present_flag;
 
 	// Slice header
-	__u8  num_ref_idx_l0_active_minus1;
-	__u8  num_ref_idx_l1_active_minus1;
+	__u32 num_ref_idx_l0_active_minus1;
+	__u32 num_ref_idx_l1_active_minus1;
 
-	__u32 reserved;
-} __attribute__((packed));
+	// Must be zero'ed
+	__u32 reserved[11];
+};
 
 #define VDE_IOCTL_BASE			('v' + 0x20)
 
-- 
2.21.0

[PATCH v1 3/4] staging: media: tegra-vde: Add IOMMU support

[Dmitry Osipenko]

All Tegra's could provide memory isolation for the video decoder
hardware using IOMMU, it is also required for Tegra30+ in order
to handle sparse dmabuf's which GPU exports in a default kernel
configuration.

Inspired-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/Kconfig       |   1 +
 drivers/staging/media/tegra-vde/Makefile      |   1 +
 drivers/staging/media/tegra-vde/iommu.c       | 148 ++++++++++++++
 drivers/staging/media/tegra-vde/trace.h       |   1 +
 .../media/tegra-vde/{tegra-vde.c => vde.c}    | 188 +++++++++---------
 drivers/staging/media/tegra-vde/vde.h         |  89 +++++++++
 6 files changed, 335 insertions(+), 93 deletions(-)
 create mode 100644 drivers/staging/media/tegra-vde/iommu.c
 rename drivers/staging/media/tegra-vde/{tegra-vde.c => vde.c} (91%)
 create mode 100644 drivers/staging/media/tegra-vde/vde.h

diff --git a/drivers/staging/media/tegra-vde/Kconfig b/drivers/staging/media/tegra-vde/Kconfig
index ff8e846cd15d..2e7f644ae591 100644
--- a/drivers/staging/media/tegra-vde/Kconfig
+++ b/drivers/staging/media/tegra-vde/Kconfig
@@ -3,6 +3,7 @@ config TEGRA_VDE
 	tristate "NVIDIA Tegra Video Decoder Engine driver"
 	depends on ARCH_TEGRA || COMPILE_TEST
 	select DMA_SHARED_BUFFER
+	select IOMMU_IOVA if IOMMU_SUPPORT
 	select SRAM
 	help
 	    Say Y here to enable support for the NVIDIA Tegra video decoder
diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
index 7f9020e634f3..c11867e28233 100644
--- a/drivers/staging/media/tegra-vde/Makefile
+++ b/drivers/staging/media/tegra-vde/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
+tegra-vde-y := vde.o iommu.o
 obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
new file mode 100644
index 000000000000..295c3d7cccd3
--- /dev/null
+++ b/drivers/staging/media/tegra-vde/iommu.c
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NVIDIA Tegra Video decoder driver
+ *
+ * Copyright (C) 2016-2019 GRATE-DRIVER project
+ */
+
+#include <linux/iommu.h>
+#include <linux/iova.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+
+#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
+#include <asm/dma-iommu.h>
+#endif
+
+#include "vde.h"
+
+int tegra_vde_iommu_map(struct tegra_vde *vde,
+			struct sg_table *sgt,
+			struct iova **iovap,
+			dma_addr_t *addrp,
+			size_t size)
+{
+	struct iova *iova;
+	unsigned long shift;
+	unsigned long end;
+	dma_addr_t addr;
+
+	end = vde->domain->geometry.aperture_end;
+	size = iova_align(&vde->iova, size);
+	shift = iova_shift(&vde->iova);
+
+	iova = alloc_iova(&vde->iova, size >> shift, end >> shift, true);
+	if (!iova)
+		return -ENOMEM;
+
+	addr = iova_dma_addr(&vde->iova, iova);
+
+	size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
+			    IOMMU_READ | IOMMU_WRITE);
+	if (!size) {
+		__free_iova(&vde->iova, iova);
+		return -ENXIO;
+	}
+
+	*iovap = iova;
+	*addrp = addr;
+
+	return 0;
+}
+
+void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova)
+{
+	unsigned long shift = iova_shift(&vde->iova);
+	unsigned long size = iova_size(iova) << shift;
+	dma_addr_t addr = iova_dma_addr(&vde->iova, iova);
+
+	iommu_unmap(vde->domain, addr, size);
+	__free_iova(&vde->iova, iova);
+}
+
+int tegra_vde_iommu_init(struct tegra_vde *vde)
+{
+	struct iova *iova;
+	unsigned long order;
+	unsigned long shift;
+	int err;
+
+	vde->group = iommu_group_get(vde->miscdev.parent);
+	if (!vde->group)
+		return 0;
+
+#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
+	if (dev->archdata.mapping) {
+		struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
+
+		arm_iommu_detach_device(dev);
+		arm_iommu_release_mapping(mapping);
+	}
+#endif
+	vde->domain = iommu_domain_alloc(&platform_bus_type);
+	if (!vde->domain) {
+		err = -ENOMEM;
+		goto put_group;
+	}
+
+	err = iova_cache_get();
+	if (err)
+		goto free_domain;
+
+	order = __ffs(vde->domain->pgsize_bitmap);
+	init_iova_domain(&vde->iova, 1UL << order, 0);
+
+	err = iommu_attach_group(vde->domain, vde->group);
+	if (err)
+		goto put_iova;
+
+	/*
+	 * We're using some static addresses that are not accessible by VDE
+	 * to trap invalid memory accesses.
+	 */
+	shift = iova_shift(&vde->iova);
+	iova = reserve_iova(&vde->iova, 0x60000000 >> shift,
+			    0x70000000 >> shift);
+	if (!iova) {
+		err = -ENOMEM;
+		goto detach_group;
+	}
+
+	/*
+	 * BSEV's end addresses wraps around due to integer overflow on
+	 * hardware context set up if IOVA is allocated at the end of
+	 * address space and VDE can't handle that. Hence simply reserve
+	 * the last page to avoid the problem.
+	 */
+	iova = reserve_iova(&vde->iova, (0xffffffff >> shift) - 1,
+			    0xffffffff >> shift);
+	if (!iova) {
+		err = -ENOMEM;
+		goto detach_group;
+	}
+
+	return 0;
+
+detach_group:
+	iommu_detach_group(vde->domain, vde->group);
+put_iova:
+	put_iova_domain(&vde->iova);
+	iova_cache_put();
+free_domain:
+	iommu_domain_free(vde->domain);
+put_group:
+	iommu_group_put(vde->group);
+
+	return err;
+}
+
+void tegra_vde_iommu_deinit(struct tegra_vde *vde)
+{
+	if (vde->domain) {
+		iommu_detach_group(vde->domain, vde->group);
+		put_iova_domain(&vde->iova);
+		iova_cache_put();
+		iommu_domain_free(vde->domain);
+		iommu_group_put(vde->group);
+	}
+}
diff --git a/drivers/staging/media/tegra-vde/trace.h b/drivers/staging/media/tegra-vde/trace.h
index 85e2f7e2d4d0..c7e7d6f5fd4c 100644
--- a/drivers/staging/media/tegra-vde/trace.h
+++ b/drivers/staging/media/tegra-vde/trace.h
@@ -7,6 +7,7 @@
 #define TEGRA_VDE_TRACE_H
 
 #include <linux/tracepoint.h>
+#include "vde.h"
 
 DECLARE_EVENT_CLASS(register_access,
 	TP_PROTO(struct tegra_vde *vde, void __iomem *base,
diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/vde.c
similarity index 91%
rename from drivers/staging/media/tegra-vde/tegra-vde.c
rename to drivers/staging/media/tegra-vde/vde.c
index cc4244da2705..cbcdbfef072d 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -22,6 +22,10 @@
 #include <soc/tegra/pmc.h>
 
 #include "uapi.h"
+#include "vde.h"
+
+#define CREATE_TRACE_POINTS
+#include "trace.h"
 
 #define ICMDQUE_WR		0x00
 #define CMDQUE_CONTROL		0x08
@@ -33,6 +37,10 @@
 #define BSE_DMA_BUSY		BIT(23)
 
 struct video_frame {
+	struct iova *y_iova;
+	struct iova *cb_iova;
+	struct iova *cr_iova;
+	struct iova *aux_iova;
 	struct dma_buf_attachment *y_dmabuf_attachment;
 	struct dma_buf_attachment *cb_dmabuf_attachment;
 	struct dma_buf_attachment *cr_dmabuf_attachment;
@@ -49,63 +57,6 @@ struct video_frame {
 	u32 flags;
 };
 
-struct tegra_vde {
-	void __iomem *sxe;
-	void __iomem *bsev;
-	void __iomem *mbe;
-	void __iomem *ppe;
-	void __iomem *mce;
-	void __iomem *tfe;
-	void __iomem *ppb;
-	void __iomem *vdma;
-	void __iomem *frameid;
-	struct mutex lock;
-	struct miscdevice miscdev;
-	struct reset_control *rst;
-	struct reset_control *rst_mc;
-	struct gen_pool *iram_pool;
-	struct completion decode_completion;
-	struct clk *clk;
-	dma_addr_t iram_lists_addr;
-	u32 *iram;
-};
-
-static __maybe_unused char const *
-tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
-{
-	if (vde->sxe == base)
-		return "SXE";
-
-	if (vde->bsev == base)
-		return "BSEV";
-
-	if (vde->mbe == base)
-		return "MBE";
-
-	if (vde->ppe == base)
-		return "PPE";
-
-	if (vde->mce == base)
-		return "MCE";
-
-	if (vde->tfe == base)
-		return "TFE";
-
-	if (vde->ppb == base)
-		return "PPB";
-
-	if (vde->vdma == base)
-		return "VDMA";
-
-	if (vde->frameid == base)
-		return "FRAMEID";
-
-	return "???";
-}
-
-#define CREATE_TRACE_POINTS
-#include "trace.h"
-
 static void tegra_vde_writel(struct tegra_vde *vde,
 			     u32 value, void __iomem *base, u32 offset)
 {
@@ -543,28 +494,35 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
 			 vde->sxe, 0x00);
 }
 
-static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
+static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
+					    enum dma_data_direction dma_dir,
+					    struct dma_buf_attachment *a,
 					    struct sg_table *sgt,
-					    enum dma_data_direction dma_dir)
+					    struct iova *iova)
 {
 	struct dma_buf *dmabuf = a->dmabuf;
 
+	if (vde->domain)
+		tegra_vde_iommu_unmap(vde, iova);
+
 	dma_buf_unmap_attachment(a, sgt, dma_dir);
 	dma_buf_detach(dmabuf, a);
 	dma_buf_put(dmabuf);
 }
 
-static int tegra_vde_attach_dmabuf(struct device *dev,
+static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
 				   int fd,
 				   unsigned long offset,
 				   size_t min_size,
 				   size_t align_size,
 				   struct dma_buf_attachment **a,
-				   dma_addr_t *addr,
+				   dma_addr_t *addrp,
 				   struct sg_table **s,
+				   struct iova **iovap,
 				   size_t *size,
 				   enum dma_data_direction dma_dir)
 {
+	struct device *dev = vde->miscdev.parent;
 	struct dma_buf_attachment *attachment;
 	struct dma_buf *dmabuf;
 	struct sg_table *sgt;
@@ -602,13 +560,23 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 		goto err_detach;
 	}
 
-	if (sgt->nents != 1) {
-		dev_err(dev, "Sparse DMA region is unsupported\n");
+	if (!vde->domain && sgt->nents > 1) {
+		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
 		err = -EINVAL;
 		goto err_unmap;
 	}
 
-	*addr = sg_dma_address(sgt->sgl) + offset;
+	if (vde->domain) {
+		err = tegra_vde_iommu_map(vde, sgt, iovap, addrp, dmabuf->size);
+		if (err) {
+			dev_err(dev, "IOMMU mapping failed: %d\n", err);
+			goto err_unmap;
+		}
+	} else {
+		*addrp = sg_dma_address(sgt->sgl);
+	}
+
+	*addrp = *addrp + offset;
 	*a = attachment;
 	*s = sgt;
 
@@ -627,7 +595,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 	return err;
 }
 
-static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
+static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 					     struct video_frame *frame,
 					     struct tegra_vde_h264_frame *src,
 					     enum dma_data_direction dma_dir,
@@ -636,29 +604,32 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 {
 	int err;
 
-	err = tegra_vde_attach_dmabuf(dev, src->y_fd,
+	err = tegra_vde_attach_dmabuf(vde, src->y_fd,
 				      src->y_offset, lsize, SZ_256,
 				      &frame->y_dmabuf_attachment,
 				      &frame->y_addr,
 				      &frame->y_sgt,
+				      &frame->y_iova,
 				      NULL, dma_dir);
 	if (err)
 		return err;
 
-	err = tegra_vde_attach_dmabuf(dev, src->cb_fd,
+	err = tegra_vde_attach_dmabuf(vde, src->cb_fd,
 				      src->cb_offset, csize, SZ_256,
 				      &frame->cb_dmabuf_attachment,
 				      &frame->cb_addr,
 				      &frame->cb_sgt,
+				      &frame->cb_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_y;
 
-	err = tegra_vde_attach_dmabuf(dev, src->cr_fd,
+	err = tegra_vde_attach_dmabuf(vde, src->cr_fd,
 				      src->cr_offset, csize, SZ_256,
 				      &frame->cr_dmabuf_attachment,
 				      &frame->cr_addr,
 				      &frame->cr_sgt,
+				      &frame->cr_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_cb;
@@ -668,11 +639,12 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 		return 0;
 	}
 
-	err = tegra_vde_attach_dmabuf(dev, src->aux_fd,
+	err = tegra_vde_attach_dmabuf(vde, src->aux_fd,
 				      src->aux_offset, csize, SZ_256,
 				      &frame->aux_dmabuf_attachment,
 				      &frame->aux_addr,
 				      &frame->aux_sgt,
+				      &frame->aux_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_cr;
@@ -680,34 +652,49 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
 	return 0;
 
 err_release_cr:
-	tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
-					frame->cr_sgt, dma_dir);
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->cr_dmabuf_attachment,
+					frame->cr_sgt,
+					frame->cr_iova);
 err_release_cb:
-	tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
-					frame->cb_sgt, dma_dir);
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->cb_dmabuf_attachment,
+					frame->cb_sgt,
+					frame->cb_iova);
 err_release_y:
-	tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
-					frame->y_sgt, dma_dir);
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->y_dmabuf_attachment,
+					frame->y_sgt,
+					frame->y_iova);
 
 	return err;
 }
 
-static void tegra_vde_release_frame_dmabufs(struct video_frame *frame,
+static void tegra_vde_release_frame_dmabufs(struct tegra_vde *vde,
+					    struct video_frame *frame,
 					    enum dma_data_direction dma_dir,
 					    bool baseline_profile)
 {
 	if (!baseline_profile)
-		tegra_vde_detach_and_put_dmabuf(frame->aux_dmabuf_attachment,
-						frame->aux_sgt, dma_dir);
-
-	tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
-					frame->cr_sgt, dma_dir);
-
-	tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
-					frame->cb_sgt, dma_dir);
-
-	tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
-					frame->y_sgt, dma_dir);
+		tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+						frame->aux_dmabuf_attachment,
+						frame->aux_sgt,
+						frame->aux_iova);
+
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->cr_dmabuf_attachment,
+					frame->cr_sgt,
+					frame->cr_iova);
+
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->cb_dmabuf_attachment,
+					frame->cb_sgt,
+					frame->cb_iova);
+
+	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
+					frame->y_dmabuf_attachment,
+					frame->y_sgt,
+					frame->y_iova);
 }
 
 static int tegra_vde_validate_frame(struct device *dev,
@@ -800,6 +787,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	struct video_frame *dpb_frames;
 	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
 	struct sg_table *bitstream_sgt;
+	struct iova *bitstream_iova;
 	enum dma_data_direction dma_dir;
 	dma_addr_t bitstream_data_addr;
 	dma_addr_t bsev_ptr;
@@ -819,12 +807,13 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	if (ret)
 		return ret;
 
-	ret = tegra_vde_attach_dmabuf(dev, ctx.bitstream_data_fd,
+	ret = tegra_vde_attach_dmabuf(vde, ctx.bitstream_data_fd,
 				      ctx.bitstream_data_offset,
 				      SZ_16K, SZ_16K,
 				      &bitstream_data_dmabuf_attachment,
 				      &bitstream_data_addr,
 				      &bitstream_sgt,
+				      &bitstream_iova,
 				      &bitstream_data_size,
 				      DMA_TO_DEVICE);
 	if (ret)
@@ -866,7 +855,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 
 		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
-		ret = tegra_vde_attach_dmabufs_to_frame(dev, &dpb_frames[i],
+		ret = tegra_vde_attach_dmabufs_to_frame(vde, &dpb_frames[i],
 							&frames[i], dma_dir,
 							ctx.baseline_profile,
 							lsize, csize);
@@ -954,7 +943,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	while (i--) {
 		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
-		tegra_vde_release_frame_dmabufs(&dpb_frames[i], dma_dir,
+		tegra_vde_release_frame_dmabufs(vde, &dpb_frames[i], dma_dir,
 						ctx.baseline_profile);
 	}
 
@@ -965,8 +954,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	kfree(frames);
 
 release_bitstream_dmabuf:
-	tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
-					bitstream_sgt, DMA_TO_DEVICE);
+	tegra_vde_detach_and_put_dmabuf(vde, DMA_TO_DEVICE,
+					bitstream_data_dmabuf_attachment,
+					bitstream_sgt,
+					bitstream_iova);
 
 	return ret;
 }
@@ -1176,10 +1167,16 @@ static int tegra_vde_probe(struct platform_device *pdev)
 	vde->miscdev.fops = &tegra_vde_fops;
 	vde->miscdev.parent = dev;
 
+	err = tegra_vde_iommu_init(vde);
+	if (err) {
+		dev_err(dev, "Failed to initialize IOMMU: %d\n", err);
+		goto err_gen_free;
+	}
+
 	err = misc_register(&vde->miscdev);
 	if (err) {
 		dev_err(dev, "Failed to register misc device: %d\n", err);
-		goto err_gen_free;
+		goto err_deinit_iommu;
 	}
 
 	pm_runtime_enable(dev);
@@ -1197,6 +1194,9 @@ static int tegra_vde_probe(struct platform_device *pdev)
 err_misc_unreg:
 	misc_deregister(&vde->miscdev);
 
+err_deinit_iommu:
+	tegra_vde_iommu_deinit(vde);
+
 err_gen_free:
 	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
 		      gen_pool_size(vde->iram_pool));
@@ -1221,6 +1221,8 @@ static int tegra_vde_remove(struct platform_device *pdev)
 
 	misc_deregister(&vde->miscdev);
 
+	tegra_vde_iommu_deinit(vde);
+
 	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
 		      gen_pool_size(vde->iram_pool));
 
diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
new file mode 100644
index 000000000000..37414b7fdee1
--- /dev/null
+++ b/drivers/staging/media/tegra-vde/vde.h
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NVIDIA Tegra Video decoder driver
+ *
+ * Copyright (C) 2016-2019 GRATE-DRIVER project
+ */
+
+#ifndef TEGRA_VDE_H
+#define TEGRA_VDE_H
+
+#include <linux/completion.h>
+#include <linux/miscdevice.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/iova.h>
+
+struct clk;
+struct gen_pool;
+struct iommu_group;
+struct iommu_domain;
+struct reset_control;
+
+struct tegra_vde {
+	void __iomem *sxe;
+	void __iomem *bsev;
+	void __iomem *mbe;
+	void __iomem *ppe;
+	void __iomem *mce;
+	void __iomem *tfe;
+	void __iomem *ppb;
+	void __iomem *vdma;
+	void __iomem *frameid;
+	struct mutex lock;
+	struct miscdevice miscdev;
+	struct reset_control *rst;
+	struct reset_control *rst_mc;
+	struct gen_pool *iram_pool;
+	struct completion decode_completion;
+	struct clk *clk;
+	struct iommu_domain *domain;
+	struct iommu_group *group;
+	struct iova_domain iova;
+	dma_addr_t iram_lists_addr;
+	u32 *iram;
+};
+
+int tegra_vde_iommu_init(struct tegra_vde *vde);
+void tegra_vde_iommu_deinit(struct tegra_vde *vde);
+int tegra_vde_iommu_map(struct tegra_vde *vde,
+			struct sg_table *sgt,
+			struct iova **iovap,
+			dma_addr_t *addrp,
+			size_t size);
+void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova);
+
+static __maybe_unused char const *
+tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
+{
+	if (vde->sxe == base)
+		return "SXE";
+
+	if (vde->bsev == base)
+		return "BSEV";
+
+	if (vde->mbe == base)
+		return "MBE";
+
+	if (vde->ppe == base)
+		return "PPE";
+
+	if (vde->mce == base)
+		return "MCE";
+
+	if (vde->tfe == base)
+		return "TFE";
+
+	if (vde->ppb == base)
+		return "PPB";
+
+	if (vde->vdma == base)
+		return "VDMA";
+
+	if (vde->frameid == base)
+		return "FRAMEID";
+
+	return "???";
+}
+
+#endif /* TEGRA_VDE_H */
-- 
2.21.0

Re: [PATCH v1 3/4] staging: media: tegra-vde: Add IOMMU support

[Hans Verkuil]

On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
> All Tegra's could provide memory isolation for the video decoder
> hardware using IOMMU, it is also required for Tegra30+ in order
> to handle sparse dmabuf's which GPU exports in a default kernel
> configuration.
> 
> Inspired-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/staging/media/tegra-vde/Kconfig       |   1 +
>  drivers/staging/media/tegra-vde/Makefile      |   1 +
>  drivers/staging/media/tegra-vde/iommu.c       | 148 ++++++++++++++
>  drivers/staging/media/tegra-vde/trace.h       |   1 +
>  .../media/tegra-vde/{tegra-vde.c => vde.c}    | 188 +++++++++---------
>  drivers/staging/media/tegra-vde/vde.h         |  89 +++++++++
>  6 files changed, 335 insertions(+), 93 deletions(-)
>  create mode 100644 drivers/staging/media/tegra-vde/iommu.c
>  rename drivers/staging/media/tegra-vde/{tegra-vde.c => vde.c} (91%)
>  create mode 100644 drivers/staging/media/tegra-vde/vde.h
> 
> diff --git a/drivers/staging/media/tegra-vde/Kconfig b/drivers/staging/media/tegra-vde/Kconfig
> index ff8e846cd15d..2e7f644ae591 100644
> --- a/drivers/staging/media/tegra-vde/Kconfig
> +++ b/drivers/staging/media/tegra-vde/Kconfig
> @@ -3,6 +3,7 @@ config TEGRA_VDE
>  	tristate "NVIDIA Tegra Video Decoder Engine driver"
>  	depends on ARCH_TEGRA || COMPILE_TEST
>  	select DMA_SHARED_BUFFER
> +	select IOMMU_IOVA if IOMMU_SUPPORT
>  	select SRAM
>  	help
>  	    Say Y here to enable support for the NVIDIA Tegra video decoder
> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
> index 7f9020e634f3..c11867e28233 100644
> --- a/drivers/staging/media/tegra-vde/Makefile
> +++ b/drivers/staging/media/tegra-vde/Makefile
> @@ -1,2 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> +tegra-vde-y := vde.o iommu.o
>  obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
> diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
> new file mode 100644
> index 000000000000..295c3d7cccd3
> --- /dev/null
> +++ b/drivers/staging/media/tegra-vde/iommu.c
> @@ -0,0 +1,148 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA Tegra Video decoder driver
> + *
> + * Copyright (C) 2016-2019 GRATE-DRIVER project
> + */
> +
> +#include <linux/iommu.h>
> +#include <linux/iova.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +
> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> +#include <asm/dma-iommu.h>
> +#endif
> +
> +#include "vde.h"
> +
> +int tegra_vde_iommu_map(struct tegra_vde *vde,
> +			struct sg_table *sgt,
> +			struct iova **iovap,
> +			dma_addr_t *addrp,
> +			size_t size)
> +{
> +	struct iova *iova;
> +	unsigned long shift;
> +	unsigned long end;
> +	dma_addr_t addr;
> +
> +	end = vde->domain->geometry.aperture_end;
> +	size = iova_align(&vde->iova, size);
> +	shift = iova_shift(&vde->iova);
> +
> +	iova = alloc_iova(&vde->iova, size >> shift, end >> shift, true);
> +	if (!iova)
> +		return -ENOMEM;
> +
> +	addr = iova_dma_addr(&vde->iova, iova);
> +
> +	size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
> +			    IOMMU_READ | IOMMU_WRITE);
> +	if (!size) {
> +		__free_iova(&vde->iova, iova);
> +		return -ENXIO;
> +	}
> +
> +	*iovap = iova;
> +	*addrp = addr;
> +
> +	return 0;
> +}
> +
> +void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova)
> +{
> +	unsigned long shift = iova_shift(&vde->iova);
> +	unsigned long size = iova_size(iova) << shift;
> +	dma_addr_t addr = iova_dma_addr(&vde->iova, iova);
> +
> +	iommu_unmap(vde->domain, addr, size);
> +	__free_iova(&vde->iova, iova);
> +}
> +
> +int tegra_vde_iommu_init(struct tegra_vde *vde)
> +{
> +	struct iova *iova;
> +	unsigned long order;
> +	unsigned long shift;
> +	int err;
> +
> +	vde->group = iommu_group_get(vde->miscdev.parent);
> +	if (!vde->group)
> +		return 0;
> +
> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
> +	if (dev->archdata.mapping) {

'dev' doesn't exist, so this fails to compile!

Regards,

	Hans

> +		struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
> +
> +		arm_iommu_detach_device(dev);
> +		arm_iommu_release_mapping(mapping);
> +	}
> +#endif
> +	vde->domain = iommu_domain_alloc(&platform_bus_type);
> +	if (!vde->domain) {
> +		err = -ENOMEM;
> +		goto put_group;
> +	}
> +
> +	err = iova_cache_get();
> +	if (err)
> +		goto free_domain;
> +
> +	order = __ffs(vde->domain->pgsize_bitmap);
> +	init_iova_domain(&vde->iova, 1UL << order, 0);
> +
> +	err = iommu_attach_group(vde->domain, vde->group);
> +	if (err)
> +		goto put_iova;
> +
> +	/*
> +	 * We're using some static addresses that are not accessible by VDE
> +	 * to trap invalid memory accesses.
> +	 */
> +	shift = iova_shift(&vde->iova);
> +	iova = reserve_iova(&vde->iova, 0x60000000 >> shift,
> +			    0x70000000 >> shift);
> +	if (!iova) {
> +		err = -ENOMEM;
> +		goto detach_group;
> +	}
> +
> +	/*
> +	 * BSEV's end addresses wraps around due to integer overflow on
> +	 * hardware context set up if IOVA is allocated at the end of
> +	 * address space and VDE can't handle that. Hence simply reserve
> +	 * the last page to avoid the problem.
> +	 */
> +	iova = reserve_iova(&vde->iova, (0xffffffff >> shift) - 1,
> +			    0xffffffff >> shift);
> +	if (!iova) {
> +		err = -ENOMEM;
> +		goto detach_group;
> +	}
> +
> +	return 0;
> +
> +detach_group:
> +	iommu_detach_group(vde->domain, vde->group);
> +put_iova:
> +	put_iova_domain(&vde->iova);
> +	iova_cache_put();
> +free_domain:
> +	iommu_domain_free(vde->domain);
> +put_group:
> +	iommu_group_put(vde->group);
> +
> +	return err;
> +}
> +
> +void tegra_vde_iommu_deinit(struct tegra_vde *vde)
> +{
> +	if (vde->domain) {
> +		iommu_detach_group(vde->domain, vde->group);
> +		put_iova_domain(&vde->iova);
> +		iova_cache_put();
> +		iommu_domain_free(vde->domain);
> +		iommu_group_put(vde->group);
> +	}
> +}
> diff --git a/drivers/staging/media/tegra-vde/trace.h b/drivers/staging/media/tegra-vde/trace.h
> index 85e2f7e2d4d0..c7e7d6f5fd4c 100644
> --- a/drivers/staging/media/tegra-vde/trace.h
> +++ b/drivers/staging/media/tegra-vde/trace.h
> @@ -7,6 +7,7 @@
>  #define TEGRA_VDE_TRACE_H
>  
>  #include <linux/tracepoint.h>
> +#include "vde.h"
>  
>  DECLARE_EVENT_CLASS(register_access,
>  	TP_PROTO(struct tegra_vde *vde, void __iomem *base,
> diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c b/drivers/staging/media/tegra-vde/vde.c
> similarity index 91%
> rename from drivers/staging/media/tegra-vde/tegra-vde.c
> rename to drivers/staging/media/tegra-vde/vde.c
> index cc4244da2705..cbcdbfef072d 100644
> --- a/drivers/staging/media/tegra-vde/tegra-vde.c
> +++ b/drivers/staging/media/tegra-vde/vde.c
> @@ -22,6 +22,10 @@
>  #include <soc/tegra/pmc.h>
>  
>  #include "uapi.h"
> +#include "vde.h"
> +
> +#define CREATE_TRACE_POINTS
> +#include "trace.h"
>  
>  #define ICMDQUE_WR		0x00
>  #define CMDQUE_CONTROL		0x08
> @@ -33,6 +37,10 @@
>  #define BSE_DMA_BUSY		BIT(23)
>  
>  struct video_frame {
> +	struct iova *y_iova;
> +	struct iova *cb_iova;
> +	struct iova *cr_iova;
> +	struct iova *aux_iova;
>  	struct dma_buf_attachment *y_dmabuf_attachment;
>  	struct dma_buf_attachment *cb_dmabuf_attachment;
>  	struct dma_buf_attachment *cr_dmabuf_attachment;
> @@ -49,63 +57,6 @@ struct video_frame {
>  	u32 flags;
>  };
>  
> -struct tegra_vde {
> -	void __iomem *sxe;
> -	void __iomem *bsev;
> -	void __iomem *mbe;
> -	void __iomem *ppe;
> -	void __iomem *mce;
> -	void __iomem *tfe;
> -	void __iomem *ppb;
> -	void __iomem *vdma;
> -	void __iomem *frameid;
> -	struct mutex lock;
> -	struct miscdevice miscdev;
> -	struct reset_control *rst;
> -	struct reset_control *rst_mc;
> -	struct gen_pool *iram_pool;
> -	struct completion decode_completion;
> -	struct clk *clk;
> -	dma_addr_t iram_lists_addr;
> -	u32 *iram;
> -};
> -
> -static __maybe_unused char const *
> -tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
> -{
> -	if (vde->sxe == base)
> -		return "SXE";
> -
> -	if (vde->bsev == base)
> -		return "BSEV";
> -
> -	if (vde->mbe == base)
> -		return "MBE";
> -
> -	if (vde->ppe == base)
> -		return "PPE";
> -
> -	if (vde->mce == base)
> -		return "MCE";
> -
> -	if (vde->tfe == base)
> -		return "TFE";
> -
> -	if (vde->ppb == base)
> -		return "PPB";
> -
> -	if (vde->vdma == base)
> -		return "VDMA";
> -
> -	if (vde->frameid == base)
> -		return "FRAMEID";
> -
> -	return "???";
> -}
> -
> -#define CREATE_TRACE_POINTS
> -#include "trace.h"
> -
>  static void tegra_vde_writel(struct tegra_vde *vde,
>  			     u32 value, void __iomem *base, u32 offset)
>  {
> @@ -543,28 +494,35 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
>  			 vde->sxe, 0x00);
>  }
>  
> -static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
> +static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
> +					    enum dma_data_direction dma_dir,
> +					    struct dma_buf_attachment *a,
>  					    struct sg_table *sgt,
> -					    enum dma_data_direction dma_dir)
> +					    struct iova *iova)
>  {
>  	struct dma_buf *dmabuf = a->dmabuf;
>  
> +	if (vde->domain)
> +		tegra_vde_iommu_unmap(vde, iova);
> +
>  	dma_buf_unmap_attachment(a, sgt, dma_dir);
>  	dma_buf_detach(dmabuf, a);
>  	dma_buf_put(dmabuf);
>  }
>  
> -static int tegra_vde_attach_dmabuf(struct device *dev,
> +static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  				   int fd,
>  				   unsigned long offset,
>  				   size_t min_size,
>  				   size_t align_size,
>  				   struct dma_buf_attachment **a,
> -				   dma_addr_t *addr,
> +				   dma_addr_t *addrp,
>  				   struct sg_table **s,
> +				   struct iova **iovap,
>  				   size_t *size,
>  				   enum dma_data_direction dma_dir)
>  {
> +	struct device *dev = vde->miscdev.parent;
>  	struct dma_buf_attachment *attachment;
>  	struct dma_buf *dmabuf;
>  	struct sg_table *sgt;
> @@ -602,13 +560,23 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
>  		goto err_detach;
>  	}
>  
> -	if (sgt->nents != 1) {
> -		dev_err(dev, "Sparse DMA region is unsupported\n");
> +	if (!vde->domain && sgt->nents > 1) {
> +		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
>  		err = -EINVAL;
>  		goto err_unmap;
>  	}
>  
> -	*addr = sg_dma_address(sgt->sgl) + offset;
> +	if (vde->domain) {
> +		err = tegra_vde_iommu_map(vde, sgt, iovap, addrp, dmabuf->size);
> +		if (err) {
> +			dev_err(dev, "IOMMU mapping failed: %d\n", err);
> +			goto err_unmap;
> +		}
> +	} else {
> +		*addrp = sg_dma_address(sgt->sgl);
> +	}
> +
> +	*addrp = *addrp + offset;
>  	*a = attachment;
>  	*s = sgt;
>  
> @@ -627,7 +595,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
>  	return err;
>  }
>  
> -static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
> +static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  					     struct video_frame *frame,
>  					     struct tegra_vde_h264_frame *src,
>  					     enum dma_data_direction dma_dir,
> @@ -636,29 +604,32 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
>  {
>  	int err;
>  
> -	err = tegra_vde_attach_dmabuf(dev, src->y_fd,
> +	err = tegra_vde_attach_dmabuf(vde, src->y_fd,
>  				      src->y_offset, lsize, SZ_256,
>  				      &frame->y_dmabuf_attachment,
>  				      &frame->y_addr,
>  				      &frame->y_sgt,
> +				      &frame->y_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		return err;
>  
> -	err = tegra_vde_attach_dmabuf(dev, src->cb_fd,
> +	err = tegra_vde_attach_dmabuf(vde, src->cb_fd,
>  				      src->cb_offset, csize, SZ_256,
>  				      &frame->cb_dmabuf_attachment,
>  				      &frame->cb_addr,
>  				      &frame->cb_sgt,
> +				      &frame->cb_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_y;
>  
> -	err = tegra_vde_attach_dmabuf(dev, src->cr_fd,
> +	err = tegra_vde_attach_dmabuf(vde, src->cr_fd,
>  				      src->cr_offset, csize, SZ_256,
>  				      &frame->cr_dmabuf_attachment,
>  				      &frame->cr_addr,
>  				      &frame->cr_sgt,
> +				      &frame->cr_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cb;
> @@ -668,11 +639,12 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
>  		return 0;
>  	}
>  
> -	err = tegra_vde_attach_dmabuf(dev, src->aux_fd,
> +	err = tegra_vde_attach_dmabuf(vde, src->aux_fd,
>  				      src->aux_offset, csize, SZ_256,
>  				      &frame->aux_dmabuf_attachment,
>  				      &frame->aux_addr,
>  				      &frame->aux_sgt,
> +				      &frame->aux_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cr;
> @@ -680,34 +652,49 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
>  	return 0;
>  
>  err_release_cr:
> -	tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
> -					frame->cr_sgt, dma_dir);
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->cr_dmabuf_attachment,
> +					frame->cr_sgt,
> +					frame->cr_iova);
>  err_release_cb:
> -	tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
> -					frame->cb_sgt, dma_dir);
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->cb_dmabuf_attachment,
> +					frame->cb_sgt,
> +					frame->cb_iova);
>  err_release_y:
> -	tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
> -					frame->y_sgt, dma_dir);
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->y_dmabuf_attachment,
> +					frame->y_sgt,
> +					frame->y_iova);
>  
>  	return err;
>  }
>  
> -static void tegra_vde_release_frame_dmabufs(struct video_frame *frame,
> +static void tegra_vde_release_frame_dmabufs(struct tegra_vde *vde,
> +					    struct video_frame *frame,
>  					    enum dma_data_direction dma_dir,
>  					    bool baseline_profile)
>  {
>  	if (!baseline_profile)
> -		tegra_vde_detach_and_put_dmabuf(frame->aux_dmabuf_attachment,
> -						frame->aux_sgt, dma_dir);
> -
> -	tegra_vde_detach_and_put_dmabuf(frame->cr_dmabuf_attachment,
> -					frame->cr_sgt, dma_dir);
> -
> -	tegra_vde_detach_and_put_dmabuf(frame->cb_dmabuf_attachment,
> -					frame->cb_sgt, dma_dir);
> -
> -	tegra_vde_detach_and_put_dmabuf(frame->y_dmabuf_attachment,
> -					frame->y_sgt, dma_dir);
> +		tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +						frame->aux_dmabuf_attachment,
> +						frame->aux_sgt,
> +						frame->aux_iova);
> +
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->cr_dmabuf_attachment,
> +					frame->cr_sgt,
> +					frame->cr_iova);
> +
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->cb_dmabuf_attachment,
> +					frame->cb_sgt,
> +					frame->cb_iova);
> +
> +	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> +					frame->y_dmabuf_attachment,
> +					frame->y_sgt,
> +					frame->y_iova);
>  }
>  
>  static int tegra_vde_validate_frame(struct device *dev,
> @@ -800,6 +787,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	struct video_frame *dpb_frames;
>  	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
>  	struct sg_table *bitstream_sgt;
> +	struct iova *bitstream_iova;
>  	enum dma_data_direction dma_dir;
>  	dma_addr_t bitstream_data_addr;
>  	dma_addr_t bsev_ptr;
> @@ -819,12 +807,13 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	if (ret)
>  		return ret;
>  
> -	ret = tegra_vde_attach_dmabuf(dev, ctx.bitstream_data_fd,
> +	ret = tegra_vde_attach_dmabuf(vde, ctx.bitstream_data_fd,
>  				      ctx.bitstream_data_offset,
>  				      SZ_16K, SZ_16K,
>  				      &bitstream_data_dmabuf_attachment,
>  				      &bitstream_data_addr,
>  				      &bitstream_sgt,
> +				      &bitstream_iova,
>  				      &bitstream_data_size,
>  				      DMA_TO_DEVICE);
>  	if (ret)
> @@ -866,7 +855,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  
>  		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>  
> -		ret = tegra_vde_attach_dmabufs_to_frame(dev, &dpb_frames[i],
> +		ret = tegra_vde_attach_dmabufs_to_frame(vde, &dpb_frames[i],
>  							&frames[i], dma_dir,
>  							ctx.baseline_profile,
>  							lsize, csize);
> @@ -954,7 +943,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	while (i--) {
>  		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>  
> -		tegra_vde_release_frame_dmabufs(&dpb_frames[i], dma_dir,
> +		tegra_vde_release_frame_dmabufs(vde, &dpb_frames[i], dma_dir,
>  						ctx.baseline_profile);
>  	}
>  
> @@ -965,8 +954,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	kfree(frames);
>  
>  release_bitstream_dmabuf:
> -	tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
> -					bitstream_sgt, DMA_TO_DEVICE);
> +	tegra_vde_detach_and_put_dmabuf(vde, DMA_TO_DEVICE,
> +					bitstream_data_dmabuf_attachment,
> +					bitstream_sgt,
> +					bitstream_iova);
>  
>  	return ret;
>  }
> @@ -1176,10 +1167,16 @@ static int tegra_vde_probe(struct platform_device *pdev)
>  	vde->miscdev.fops = &tegra_vde_fops;
>  	vde->miscdev.parent = dev;
>  
> +	err = tegra_vde_iommu_init(vde);
> +	if (err) {
> +		dev_err(dev, "Failed to initialize IOMMU: %d\n", err);
> +		goto err_gen_free;
> +	}
> +
>  	err = misc_register(&vde->miscdev);
>  	if (err) {
>  		dev_err(dev, "Failed to register misc device: %d\n", err);
> -		goto err_gen_free;
> +		goto err_deinit_iommu;
>  	}
>  
>  	pm_runtime_enable(dev);
> @@ -1197,6 +1194,9 @@ static int tegra_vde_probe(struct platform_device *pdev)
>  err_misc_unreg:
>  	misc_deregister(&vde->miscdev);
>  
> +err_deinit_iommu:
> +	tegra_vde_iommu_deinit(vde);
> +
>  err_gen_free:
>  	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
>  		      gen_pool_size(vde->iram_pool));
> @@ -1221,6 +1221,8 @@ static int tegra_vde_remove(struct platform_device *pdev)
>  
>  	misc_deregister(&vde->miscdev);
>  
> +	tegra_vde_iommu_deinit(vde);
> +
>  	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
>  		      gen_pool_size(vde->iram_pool));
>  
> diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
> new file mode 100644
> index 000000000000..37414b7fdee1
> --- /dev/null
> +++ b/drivers/staging/media/tegra-vde/vde.h
> @@ -0,0 +1,89 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA Tegra Video decoder driver
> + *
> + * Copyright (C) 2016-2019 GRATE-DRIVER project
> + */
> +
> +#ifndef TEGRA_VDE_H
> +#define TEGRA_VDE_H
> +
> +#include <linux/completion.h>
> +#include <linux/miscdevice.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +#include <linux/iova.h>
> +
> +struct clk;
> +struct gen_pool;
> +struct iommu_group;
> +struct iommu_domain;
> +struct reset_control;
> +
> +struct tegra_vde {
> +	void __iomem *sxe;
> +	void __iomem *bsev;
> +	void __iomem *mbe;
> +	void __iomem *ppe;
> +	void __iomem *mce;
> +	void __iomem *tfe;
> +	void __iomem *ppb;
> +	void __iomem *vdma;
> +	void __iomem *frameid;
> +	struct mutex lock;
> +	struct miscdevice miscdev;
> +	struct reset_control *rst;
> +	struct reset_control *rst_mc;
> +	struct gen_pool *iram_pool;
> +	struct completion decode_completion;
> +	struct clk *clk;
> +	struct iommu_domain *domain;
> +	struct iommu_group *group;
> +	struct iova_domain iova;
> +	dma_addr_t iram_lists_addr;
> +	u32 *iram;
> +};
> +
> +int tegra_vde_iommu_init(struct tegra_vde *vde);
> +void tegra_vde_iommu_deinit(struct tegra_vde *vde);
> +int tegra_vde_iommu_map(struct tegra_vde *vde,
> +			struct sg_table *sgt,
> +			struct iova **iovap,
> +			dma_addr_t *addrp,
> +			size_t size);
> +void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova);
> +
> +static __maybe_unused char const *
> +tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
> +{
> +	if (vde->sxe == base)
> +		return "SXE";
> +
> +	if (vde->bsev == base)
> +		return "BSEV";
> +
> +	if (vde->mbe == base)
> +		return "MBE";
> +
> +	if (vde->ppe == base)
> +		return "PPE";
> +
> +	if (vde->mce == base)
> +		return "MCE";
> +
> +	if (vde->tfe == base)
> +		return "TFE";
> +
> +	if (vde->ppb == base)
> +		return "PPB";
> +
> +	if (vde->vdma == base)
> +		return "VDMA";
> +
> +	if (vde->frameid == base)
> +		return "FRAMEID";
> +
> +	return "???";
> +}
> +
> +#endif /* TEGRA_VDE_H */
> 

Re: [PATCH v1 3/4] staging: media: tegra-vde: Add IOMMU support

[Dmitry Osipenko]

17.06.2019 16:31, Hans Verkuil пишет:
> On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
>> All Tegra's could provide memory isolation for the video decoder
>> hardware using IOMMU, it is also required for Tegra30+ in order
>> to handle sparse dmabuf's which GPU exports in a default kernel
>> configuration.
>>
>> Inspired-by: Thierry Reding <thierry.reding@gmail.com>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/staging/media/tegra-vde/Kconfig       |   1 +
>>  drivers/staging/media/tegra-vde/Makefile      |   1 +
>>  drivers/staging/media/tegra-vde/iommu.c       | 148 ++++++++++++++
>>  drivers/staging/media/tegra-vde/trace.h       |   1 +
>>  .../media/tegra-vde/{tegra-vde.c => vde.c}    | 188 +++++++++---------
>>  drivers/staging/media/tegra-vde/vde.h         |  89 +++++++++
>>  6 files changed, 335 insertions(+), 93 deletions(-)
>>  create mode 100644 drivers/staging/media/tegra-vde/iommu.c
>>  rename drivers/staging/media/tegra-vde/{tegra-vde.c => vde.c} (91%)
>>  create mode 100644 drivers/staging/media/tegra-vde/vde.h
>>
>> diff --git a/drivers/staging/media/tegra-vde/Kconfig b/drivers/staging/media/tegra-vde/Kconfig
>> index ff8e846cd15d..2e7f644ae591 100644
>> --- a/drivers/staging/media/tegra-vde/Kconfig
>> +++ b/drivers/staging/media/tegra-vde/Kconfig
>> @@ -3,6 +3,7 @@ config TEGRA_VDE
>>  	tristate "NVIDIA Tegra Video Decoder Engine driver"
>>  	depends on ARCH_TEGRA || COMPILE_TEST
>>  	select DMA_SHARED_BUFFER
>> +	select IOMMU_IOVA if IOMMU_SUPPORT
>>  	select SRAM
>>  	help
>>  	    Say Y here to enable support for the NVIDIA Tegra video decoder
>> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
>> index 7f9020e634f3..c11867e28233 100644
>> --- a/drivers/staging/media/tegra-vde/Makefile
>> +++ b/drivers/staging/media/tegra-vde/Makefile
>> @@ -1,2 +1,3 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> +tegra-vde-y := vde.o iommu.o
>>  obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
>> diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
>> new file mode 100644
>> index 000000000000..295c3d7cccd3
>> --- /dev/null
>> +++ b/drivers/staging/media/tegra-vde/iommu.c
>> @@ -0,0 +1,148 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * NVIDIA Tegra Video decoder driver
>> + *
>> + * Copyright (C) 2016-2019 GRATE-DRIVER project
>> + */
>> +
>> +#include <linux/iommu.h>
>> +#include <linux/iova.h>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +
>> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
>> +#include <asm/dma-iommu.h>
>> +#endif
>> +
>> +#include "vde.h"
>> +
>> +int tegra_vde_iommu_map(struct tegra_vde *vde,
>> +			struct sg_table *sgt,
>> +			struct iova **iovap,
>> +			dma_addr_t *addrp,
>> +			size_t size)
>> +{
>> +	struct iova *iova;
>> +	unsigned long shift;
>> +	unsigned long end;
>> +	dma_addr_t addr;
>> +
>> +	end = vde->domain->geometry.aperture_end;
>> +	size = iova_align(&vde->iova, size);
>> +	shift = iova_shift(&vde->iova);
>> +
>> +	iova = alloc_iova(&vde->iova, size >> shift, end >> shift, true);
>> +	if (!iova)
>> +		return -ENOMEM;
>> +
>> +	addr = iova_dma_addr(&vde->iova, iova);
>> +
>> +	size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
>> +			    IOMMU_READ | IOMMU_WRITE);
>> +	if (!size) {
>> +		__free_iova(&vde->iova, iova);
>> +		return -ENXIO;
>> +	}
>> +
>> +	*iovap = iova;
>> +	*addrp = addr;
>> +
>> +	return 0;
>> +}
>> +
>> +void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova)
>> +{
>> +	unsigned long shift = iova_shift(&vde->iova);
>> +	unsigned long size = iova_size(iova) << shift;
>> +	dma_addr_t addr = iova_dma_addr(&vde->iova, iova);
>> +
>> +	iommu_unmap(vde->domain, addr, size);
>> +	__free_iova(&vde->iova, iova);
>> +}
>> +
>> +int tegra_vde_iommu_init(struct tegra_vde *vde)
>> +{
>> +	struct iova *iova;
>> +	unsigned long order;
>> +	unsigned long shift;
>> +	int err;
>> +
>> +	vde->group = iommu_group_get(vde->miscdev.parent);
>> +	if (!vde->group)
>> +		return 0;
>> +
>> +#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
>> +	if (dev->archdata.mapping) {
> 
> 'dev' doesn't exist, so this fails to compile!

Oh, indeed! I actually didn't even try to compile with CONFIG_ARM_DMA_USE_IOMMU. Will
fix it in v2, thanks!

[PATCH v1 4/4] staging: media: tegra-vde: Defer dmabuf's unmapping

[Dmitry Osipenko]

Frequent IOMMU remappings take about 50% of CPU usage because there is
quite a lot to remap. Defer dmabuf's unmapping by 5 seconds in order to
mitigate the mapping overhead which goes away completely and driver works
as fast as in a case of a disabled IOMMU. The case of a disabled IOMMU
should also benefit a tad from the caching since CPU cache maintenance
that happens on dmabuf's attaching takes some resources.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/staging/media/tegra-vde/Makefile      |   2 +-
 .../staging/media/tegra-vde/dmabuf-cache.c    | 223 ++++++++++++++++++
 drivers/staging/media/tegra-vde/iommu.c       |   2 -
 drivers/staging/media/tegra-vde/vde.c         | 143 +++--------
 drivers/staging/media/tegra-vde/vde.h         |  18 +-
 5 files changed, 273 insertions(+), 115 deletions(-)
 create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c

diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
index c11867e28233..2827f7601de8 100644
--- a/drivers/staging/media/tegra-vde/Makefile
+++ b/drivers/staging/media/tegra-vde/Makefile
@@ -1,3 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0
-tegra-vde-y := vde.o iommu.o
+tegra-vde-y := vde.o iommu.o dmabuf-cache.o
 obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
diff --git a/drivers/staging/media/tegra-vde/dmabuf-cache.c b/drivers/staging/media/tegra-vde/dmabuf-cache.c
new file mode 100644
index 000000000000..fcde8d1c37e7
--- /dev/null
+++ b/drivers/staging/media/tegra-vde/dmabuf-cache.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * NVIDIA Tegra Video decoder driver
+ *
+ * Copyright (C) 2016-2019 GRATE-DRIVER project
+ */
+
+#include <linux/dma-buf.h>
+#include <linux/iova.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+
+#include "vde.h"
+
+struct tegra_vde_cache_entry {
+	enum dma_data_direction dma_dir;
+	struct dma_buf_attachment *a;
+	struct delayed_work dwork;
+	struct tegra_vde *vde;
+	struct list_head list;
+	struct sg_table *sgt;
+	struct iova *iova;
+	unsigned int refcnt;
+};
+
+static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry)
+{
+	struct dma_buf *dmabuf = entry->a->dmabuf;
+
+	WARN_ON_ONCE(entry->refcnt);
+
+	if (entry->vde->domain)
+		tegra_vde_iommu_unmap(entry->vde, entry->iova);
+
+	dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
+	dma_buf_detach(dmabuf, entry->a);
+	dma_buf_put(dmabuf);
+
+	list_del(&entry->list);
+	kfree(entry);
+}
+
+static void tegra_vde_delayed_unmap(struct work_struct *work)
+{
+	struct tegra_vde_cache_entry *entry;
+
+	entry = container_of(work, struct tegra_vde_cache_entry,
+			     dwork.work);
+
+	mutex_lock(&entry->vde->map_lock);
+	tegra_vde_release_entry(entry);
+	mutex_unlock(&entry->vde->map_lock);
+}
+
+int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
+			       struct dma_buf *dmabuf,
+			       enum dma_data_direction dma_dir,
+			       struct dma_buf_attachment **ap,
+			       dma_addr_t *addrp)
+{
+	struct device *dev = vde->miscdev.parent;
+	struct tegra_vde_cache_entry *entry;
+	struct dma_buf_attachment *attachment;
+	struct sg_table *sgt;
+	struct iova *iova;
+	int err;
+
+	mutex_lock(&vde->map_lock);
+
+	list_for_each_entry(entry, &vde->map_list, list) {
+		if (entry->a->dmabuf != dmabuf)
+			continue;
+
+		if (!cancel_delayed_work(&entry->dwork))
+			continue;
+
+		if (entry->dma_dir != dma_dir)
+			entry->dma_dir = DMA_BIDIRECTIONAL;
+
+		dma_buf_put(dmabuf);
+
+		if (vde->domain)
+			*addrp = iova_dma_addr(&vde->iova, entry->iova);
+		else
+			*addrp = sg_dma_address(entry->sgt->sgl);
+
+		goto ref;
+	}
+
+	attachment = dma_buf_attach(dmabuf, dev);
+	if (IS_ERR(attachment)) {
+		dev_err(dev, "Failed to attach dmabuf\n");
+		err = PTR_ERR(attachment);
+		goto err_unlock;
+	}
+
+	sgt = dma_buf_map_attachment(attachment, dma_dir);
+	if (IS_ERR(sgt)) {
+		dev_err(dev, "Failed to get dmabufs sg_table\n");
+		err = PTR_ERR(sgt);
+		goto err_detach;
+	}
+
+	if (!vde->domain && sgt->nents > 1) {
+		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
+		err = -EINVAL;
+		goto err_unmap;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry) {
+		err = -ENOMEM;
+		goto err_unmap;
+	}
+
+	if (vde->domain) {
+		err = tegra_vde_iommu_map(vde, sgt, &iova, dmabuf->size);
+		if (err)
+			goto err_free;
+
+		*addrp = iova_dma_addr(&vde->iova, iova);
+	} else {
+		*addrp = sg_dma_address(sgt->sgl);
+	}
+
+	INIT_DELAYED_WORK(&entry->dwork, tegra_vde_delayed_unmap);
+	list_add(&entry->list, &vde->map_list);
+
+	entry->dma_dir = dma_dir;
+	entry->iova = iova;
+	entry->vde = vde;
+	entry->sgt = sgt;
+	entry->a = attachment;
+ref:
+	entry->refcnt++;
+
+	*ap = entry->a;
+
+	mutex_unlock(&vde->map_lock);
+
+	return 0;
+
+err_free:
+	kfree(entry);
+err_unmap:
+	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
+err_detach:
+	dma_buf_detach(dmabuf, attachment);
+err_unlock:
+	mutex_unlock(&vde->map_lock);
+
+	return err;
+}
+
+void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
+				  struct dma_buf_attachment *a,
+				  bool release)
+{
+	struct tegra_vde_cache_entry *entry;
+
+	mutex_lock(&vde->map_lock);
+
+	list_for_each_entry(entry, &vde->map_list, list) {
+		if (entry->a != a)
+			continue;
+
+		WARN_ON_ONCE(!entry->refcnt);
+
+		if (--entry->refcnt == 0) {
+			if (release)
+				tegra_vde_release_entry(entry);
+			else
+				schedule_delayed_work(&entry->dwork, 5 * HZ);
+		}
+		break;
+	}
+
+	mutex_unlock(&vde->map_lock);
+}
+
+void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde)
+{
+	struct tegra_vde_cache_entry *entry, *tmp;
+
+	mutex_lock(&vde->map_lock);
+
+	list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
+		if (entry->refcnt)
+			continue;
+
+		if (!cancel_delayed_work(&entry->dwork))
+			continue;
+
+		tegra_vde_release_entry(entry);
+	}
+
+	mutex_unlock(&vde->map_lock);
+}
+
+void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde)
+{
+	struct tegra_vde_cache_entry *entry, *tmp;
+
+	mutex_lock(&vde->map_lock);
+
+	do {
+		list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
+			if (!cancel_delayed_work(&entry->dwork))
+				continue;
+
+			tegra_vde_release_entry(entry);
+		}
+
+		mutex_unlock(&vde->map_lock);
+		schedule();
+		mutex_lock(&vde->map_lock);
+	} while (!list_empty(&vde->map_list));
+
+	mutex_unlock(&vde->map_lock);
+}
diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
index 295c3d7cccd3..6d635332e0ec 100644
--- a/drivers/staging/media/tegra-vde/iommu.c
+++ b/drivers/staging/media/tegra-vde/iommu.c
@@ -19,7 +19,6 @@
 int tegra_vde_iommu_map(struct tegra_vde *vde,
 			struct sg_table *sgt,
 			struct iova **iovap,
-			dma_addr_t *addrp,
 			size_t size)
 {
 	struct iova *iova;
@@ -45,7 +44,6 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
 	}
 
 	*iovap = iova;
-	*addrp = addr;
 
 	return 0;
 }
diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
index cbcdbfef072d..3466daddf663 100644
--- a/drivers/staging/media/tegra-vde/vde.c
+++ b/drivers/staging/media/tegra-vde/vde.c
@@ -11,6 +11,7 @@
 #include <linux/genalloc.h>
 #include <linux/interrupt.h>
 #include <linux/iopoll.h>
+#include <linux/list.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
@@ -37,18 +38,10 @@
 #define BSE_DMA_BUSY		BIT(23)
 
 struct video_frame {
-	struct iova *y_iova;
-	struct iova *cb_iova;
-	struct iova *cr_iova;
-	struct iova *aux_iova;
 	struct dma_buf_attachment *y_dmabuf_attachment;
 	struct dma_buf_attachment *cb_dmabuf_attachment;
 	struct dma_buf_attachment *cr_dmabuf_attachment;
 	struct dma_buf_attachment *aux_dmabuf_attachment;
-	struct sg_table *y_sgt;
-	struct sg_table *cb_sgt;
-	struct sg_table *cr_sgt;
-	struct sg_table *aux_sgt;
 	dma_addr_t y_addr;
 	dma_addr_t cb_addr;
 	dma_addr_t cr_addr;
@@ -494,22 +487,6 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
 			 vde->sxe, 0x00);
 }
 
-static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
-					    enum dma_data_direction dma_dir,
-					    struct dma_buf_attachment *a,
-					    struct sg_table *sgt,
-					    struct iova *iova)
-{
-	struct dma_buf *dmabuf = a->dmabuf;
-
-	if (vde->domain)
-		tegra_vde_iommu_unmap(vde, iova);
-
-	dma_buf_unmap_attachment(a, sgt, dma_dir);
-	dma_buf_detach(dmabuf, a);
-	dma_buf_put(dmabuf);
-}
-
 static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
 				   int fd,
 				   unsigned long offset,
@@ -517,15 +494,11 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
 				   size_t align_size,
 				   struct dma_buf_attachment **a,
 				   dma_addr_t *addrp,
-				   struct sg_table **s,
-				   struct iova **iovap,
 				   size_t *size,
 				   enum dma_data_direction dma_dir)
 {
 	struct device *dev = vde->miscdev.parent;
-	struct dma_buf_attachment *attachment;
 	struct dma_buf *dmabuf;
-	struct sg_table *sgt;
 	int err;
 
 	dmabuf = dma_buf_get(fd);
@@ -546,49 +519,17 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
 		return -EINVAL;
 	}
 
-	attachment = dma_buf_attach(dmabuf, dev);
-	if (IS_ERR(attachment)) {
-		dev_err(dev, "Failed to attach dmabuf\n");
-		err = PTR_ERR(attachment);
+	err = tegra_vde_dmabuf_cache_map(vde, dmabuf, dma_dir, a, addrp);
+	if (err)
 		goto err_put;
-	}
-
-	sgt = dma_buf_map_attachment(attachment, dma_dir);
-	if (IS_ERR(sgt)) {
-		dev_err(dev, "Failed to get dmabufs sg_table\n");
-		err = PTR_ERR(sgt);
-		goto err_detach;
-	}
-
-	if (!vde->domain && sgt->nents > 1) {
-		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
-		err = -EINVAL;
-		goto err_unmap;
-	}
-
-	if (vde->domain) {
-		err = tegra_vde_iommu_map(vde, sgt, iovap, addrp, dmabuf->size);
-		if (err) {
-			dev_err(dev, "IOMMU mapping failed: %d\n", err);
-			goto err_unmap;
-		}
-	} else {
-		*addrp = sg_dma_address(sgt->sgl);
-	}
 
 	*addrp = *addrp + offset;
-	*a = attachment;
-	*s = sgt;
 
 	if (size)
 		*size = dmabuf->size - offset;
 
 	return 0;
 
-err_unmap:
-	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
-err_detach:
-	dma_buf_detach(dmabuf, attachment);
 err_put:
 	dma_buf_put(dmabuf);
 
@@ -608,8 +549,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 				      src->y_offset, lsize, SZ_256,
 				      &frame->y_dmabuf_attachment,
 				      &frame->y_addr,
-				      &frame->y_sgt,
-				      &frame->y_iova,
 				      NULL, dma_dir);
 	if (err)
 		return err;
@@ -618,8 +557,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 				      src->cb_offset, csize, SZ_256,
 				      &frame->cb_dmabuf_attachment,
 				      &frame->cb_addr,
-				      &frame->cb_sgt,
-				      &frame->cb_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_y;
@@ -628,8 +565,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 				      src->cr_offset, csize, SZ_256,
 				      &frame->cr_dmabuf_attachment,
 				      &frame->cr_addr,
-				      &frame->cr_sgt,
-				      &frame->cr_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_cb;
@@ -643,8 +578,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 				      src->aux_offset, csize, SZ_256,
 				      &frame->aux_dmabuf_attachment,
 				      &frame->aux_addr,
-				      &frame->aux_sgt,
-				      &frame->aux_iova,
 				      NULL, dma_dir);
 	if (err)
 		goto err_release_cr;
@@ -652,20 +585,11 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 	return 0;
 
 err_release_cr:
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->cr_dmabuf_attachment,
-					frame->cr_sgt,
-					frame->cr_iova);
+	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, true);
 err_release_cb:
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->cb_dmabuf_attachment,
-					frame->cb_sgt,
-					frame->cb_iova);
+	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, true);
 err_release_y:
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->y_dmabuf_attachment,
-					frame->y_sgt,
-					frame->y_iova);
+	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, true);
 
 	return err;
 }
@@ -673,28 +597,16 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 static void tegra_vde_release_frame_dmabufs(struct tegra_vde *vde,
 					    struct video_frame *frame,
 					    enum dma_data_direction dma_dir,
-					    bool baseline_profile)
+					    bool baseline_profile,
+					    bool release)
 {
 	if (!baseline_profile)
-		tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-						frame->aux_dmabuf_attachment,
-						frame->aux_sgt,
-						frame->aux_iova);
-
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->cr_dmabuf_attachment,
-					frame->cr_sgt,
-					frame->cr_iova);
-
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->cb_dmabuf_attachment,
-					frame->cb_sgt,
-					frame->cb_iova);
-
-	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
-					frame->y_dmabuf_attachment,
-					frame->y_sgt,
-					frame->y_iova);
+		tegra_vde_dmabuf_cache_unmap(vde, frame->aux_dmabuf_attachment,
+					     release);
+
+	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, release);
+	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, release);
+	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, release);
 }
 
 static int tegra_vde_validate_frame(struct device *dev,
@@ -786,8 +698,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	struct tegra_vde_h264_frame __user *frames_user;
 	struct video_frame *dpb_frames;
 	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
-	struct sg_table *bitstream_sgt;
-	struct iova *bitstream_iova;
 	enum dma_data_direction dma_dir;
 	dma_addr_t bitstream_data_addr;
 	dma_addr_t bsev_ptr;
@@ -812,8 +722,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 				      SZ_16K, SZ_16K,
 				      &bitstream_data_dmabuf_attachment,
 				      &bitstream_data_addr,
-				      &bitstream_sgt,
-				      &bitstream_iova,
 				      &bitstream_data_size,
 				      DMA_TO_DEVICE);
 	if (ret)
@@ -944,7 +852,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
 		tegra_vde_release_frame_dmabufs(vde, &dpb_frames[i], dma_dir,
-						ctx.baseline_profile);
+						ctx.baseline_profile, ret != 0);
 	}
 
 free_dpb_frames:
@@ -954,10 +862,8 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
 	kfree(frames);
 
 release_bitstream_dmabuf:
-	tegra_vde_detach_and_put_dmabuf(vde, DMA_TO_DEVICE,
-					bitstream_data_dmabuf_attachment,
-					bitstream_sgt,
-					bitstream_iova);
+	tegra_vde_dmabuf_cache_unmap(vde, bitstream_data_dmabuf_attachment,
+				     ret != 0);
 
 	return ret;
 }
@@ -979,9 +885,21 @@ static long tegra_vde_unlocked_ioctl(struct file *filp,
 	return -ENOTTY;
 }
 
+static int tegra_vde_release_file(struct inode *inode, struct file *filp)
+{
+	struct miscdevice *miscdev = filp->private_data;
+	struct tegra_vde *vde = container_of(miscdev, struct tegra_vde,
+					     miscdev);
+
+	tegra_vde_dmabuf_cache_unmap_sync(vde);
+
+	return 0;
+}
+
 static const struct file_operations tegra_vde_fops = {
 	.owner		= THIS_MODULE,
 	.unlocked_ioctl	= tegra_vde_unlocked_ioctl,
+	.release	= tegra_vde_release_file,
 };
 
 static irqreturn_t tegra_vde_isr(int irq, void *data)
@@ -1159,6 +1077,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	INIT_LIST_HEAD(&vde->map_list);
+	mutex_init(&vde->map_lock);
 	mutex_init(&vde->lock);
 	init_completion(&vde->decode_completion);
 
@@ -1221,6 +1141,7 @@ static int tegra_vde_remove(struct platform_device *pdev)
 
 	misc_deregister(&vde->miscdev);
 
+	tegra_vde_dmabuf_cache_unmap_all(vde);
 	tegra_vde_iommu_deinit(vde);
 
 	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
index 37414b7fdee1..e138878e8e14 100644
--- a/drivers/staging/media/tegra-vde/vde.h
+++ b/drivers/staging/media/tegra-vde/vde.h
@@ -9,16 +9,20 @@
 #define TEGRA_VDE_H
 
 #include <linux/completion.h>
+#include <linux/dma-direction.h>
+#include <linux/list.h>
 #include <linux/miscdevice.h>
 #include <linux/mutex.h>
 #include <linux/types.h>
 #include <linux/iova.h>
 
 struct clk;
+struct dma_buf;
 struct gen_pool;
 struct iommu_group;
 struct iommu_domain;
 struct reset_control;
+struct dma_buf_attachment;
 
 struct tegra_vde {
 	void __iomem *sxe;
@@ -31,6 +35,8 @@ struct tegra_vde {
 	void __iomem *vdma;
 	void __iomem *frameid;
 	struct mutex lock;
+	struct mutex map_lock;
+	struct list_head map_list;
 	struct miscdevice miscdev;
 	struct reset_control *rst;
 	struct reset_control *rst_mc;
@@ -49,10 +55,20 @@ void tegra_vde_iommu_deinit(struct tegra_vde *vde);
 int tegra_vde_iommu_map(struct tegra_vde *vde,
 			struct sg_table *sgt,
 			struct iova **iovap,
-			dma_addr_t *addrp,
 			size_t size);
 void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova);
 
+int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
+			       struct dma_buf *dmabuf,
+			       enum dma_data_direction dma_dir,
+			       struct dma_buf_attachment **ap,
+			       dma_addr_t *addrp);
+void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
+				  struct dma_buf_attachment *a,
+				  bool release);
+void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde);
+void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde);
+
 static __maybe_unused char const *
 tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
 {
-- 
2.21.0

Re: [PATCH v1 4/4] staging: media: tegra-vde: Defer dmabuf's unmapping

[Hans Verkuil]

On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
> Frequent IOMMU remappings take about 50% of CPU usage because there is
> quite a lot to remap. Defer dmabuf's unmapping by 5 seconds in order to
> mitigate the mapping overhead which goes away completely and driver works
> as fast as in a case of a disabled IOMMU. The case of a disabled IOMMU
> should also benefit a tad from the caching since CPU cache maintenance
> that happens on dmabuf's attaching takes some resources.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/staging/media/tegra-vde/Makefile      |   2 +-
>  .../staging/media/tegra-vde/dmabuf-cache.c    | 223 ++++++++++++++++++
>  drivers/staging/media/tegra-vde/iommu.c       |   2 -
>  drivers/staging/media/tegra-vde/vde.c         | 143 +++--------
>  drivers/staging/media/tegra-vde/vde.h         |  18 +-
>  5 files changed, 273 insertions(+), 115 deletions(-)
>  create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c
> 
> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
> index c11867e28233..2827f7601de8 100644
> --- a/drivers/staging/media/tegra-vde/Makefile
> +++ b/drivers/staging/media/tegra-vde/Makefile
> @@ -1,3 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> -tegra-vde-y := vde.o iommu.o
> +tegra-vde-y := vde.o iommu.o dmabuf-cache.o
>  obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
> diff --git a/drivers/staging/media/tegra-vde/dmabuf-cache.c b/drivers/staging/media/tegra-vde/dmabuf-cache.c
> new file mode 100644
> index 000000000000..fcde8d1c37e7
> --- /dev/null
> +++ b/drivers/staging/media/tegra-vde/dmabuf-cache.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA Tegra Video decoder driver
> + *
> + * Copyright (C) 2016-2019 GRATE-DRIVER project
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/iova.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#include "vde.h"
> +
> +struct tegra_vde_cache_entry {
> +	enum dma_data_direction dma_dir;
> +	struct dma_buf_attachment *a;
> +	struct delayed_work dwork;
> +	struct tegra_vde *vde;
> +	struct list_head list;
> +	struct sg_table *sgt;
> +	struct iova *iova;
> +	unsigned int refcnt;
> +};
> +
> +static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry)
> +{
> +	struct dma_buf *dmabuf = entry->a->dmabuf;
> +
> +	WARN_ON_ONCE(entry->refcnt);
> +
> +	if (entry->vde->domain)
> +		tegra_vde_iommu_unmap(entry->vde, entry->iova);
> +
> +	dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
> +	dma_buf_detach(dmabuf, entry->a);
> +	dma_buf_put(dmabuf);
> +
> +	list_del(&entry->list);
> +	kfree(entry);
> +}
> +
> +static void tegra_vde_delayed_unmap(struct work_struct *work)
> +{
> +	struct tegra_vde_cache_entry *entry;
> +
> +	entry = container_of(work, struct tegra_vde_cache_entry,
> +			     dwork.work);
> +
> +	mutex_lock(&entry->vde->map_lock);
> +	tegra_vde_release_entry(entry);
> +	mutex_unlock(&entry->vde->map_lock);

From smatch:

drivers/staging/media/tegra-vde/dmabuf-cache.c:55 tegra_vde_delayed_unmap() error: dereferencing freed memory 'entry'

> +}
> +
> +int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
> +			       struct dma_buf *dmabuf,
> +			       enum dma_data_direction dma_dir,
> +			       struct dma_buf_attachment **ap,
> +			       dma_addr_t *addrp)
> +{
> +	struct device *dev = vde->miscdev.parent;
> +	struct tegra_vde_cache_entry *entry;
> +	struct dma_buf_attachment *attachment;
> +	struct sg_table *sgt;
> +	struct iova *iova;
> +	int err;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry(entry, &vde->map_list, list) {
> +		if (entry->a->dmabuf != dmabuf)
> +			continue;
> +
> +		if (!cancel_delayed_work(&entry->dwork))
> +			continue;
> +
> +		if (entry->dma_dir != dma_dir)
> +			entry->dma_dir = DMA_BIDIRECTIONAL;
> +
> +		dma_buf_put(dmabuf);
> +
> +		if (vde->domain)
> +			*addrp = iova_dma_addr(&vde->iova, entry->iova);
> +		else
> +			*addrp = sg_dma_address(entry->sgt->sgl);
> +
> +		goto ref;
> +	}
> +
> +	attachment = dma_buf_attach(dmabuf, dev);
> +	if (IS_ERR(attachment)) {
> +		dev_err(dev, "Failed to attach dmabuf\n");
> +		err = PTR_ERR(attachment);
> +		goto err_unlock;
> +	}
> +
> +	sgt = dma_buf_map_attachment(attachment, dma_dir);
> +	if (IS_ERR(sgt)) {
> +		dev_err(dev, "Failed to get dmabufs sg_table\n");
> +		err = PTR_ERR(sgt);
> +		goto err_detach;
> +	}
> +
> +	if (!vde->domain && sgt->nents > 1) {
> +		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
> +		err = -EINVAL;
> +		goto err_unmap;
> +	}
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry) {
> +		err = -ENOMEM;
> +		goto err_unmap;
> +	}
> +
> +	if (vde->domain) {
> +		err = tegra_vde_iommu_map(vde, sgt, &iova, dmabuf->size);
> +		if (err)
> +			goto err_free;
> +
> +		*addrp = iova_dma_addr(&vde->iova, iova);
> +	} else {
> +		*addrp = sg_dma_address(sgt->sgl);
> +	}
> +
> +	INIT_DELAYED_WORK(&entry->dwork, tegra_vde_delayed_unmap);
> +	list_add(&entry->list, &vde->map_list);
> +
> +	entry->dma_dir = dma_dir;
> +	entry->iova = iova;

From smatch:

drivers/staging/media/tegra-vde/dmabuf-cache.c:133 tegra_vde_dmabuf_cache_map() error: uninitialized symbol 'iova'.

Regards,

	Hans

> +	entry->vde = vde;
> +	entry->sgt = sgt;
> +	entry->a = attachment;
> +ref:
> +	entry->refcnt++;
> +
> +	*ap = entry->a;
> +
> +	mutex_unlock(&vde->map_lock);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(entry);
> +err_unmap:
> +	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
> +err_detach:
> +	dma_buf_detach(dmabuf, attachment);
> +err_unlock:
> +	mutex_unlock(&vde->map_lock);
> +
> +	return err;
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
> +				  struct dma_buf_attachment *a,
> +				  bool release)
> +{
> +	struct tegra_vde_cache_entry *entry;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry(entry, &vde->map_list, list) {
> +		if (entry->a != a)
> +			continue;
> +
> +		WARN_ON_ONCE(!entry->refcnt);
> +
> +		if (--entry->refcnt == 0) {
> +			if (release)
> +				tegra_vde_release_entry(entry);
> +			else
> +				schedule_delayed_work(&entry->dwork, 5 * HZ);
> +		}
> +		break;
> +	}
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde)
> +{
> +	struct tegra_vde_cache_entry *entry, *tmp;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
> +		if (entry->refcnt)
> +			continue;
> +
> +		if (!cancel_delayed_work(&entry->dwork))
> +			continue;
> +
> +		tegra_vde_release_entry(entry);
> +	}
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde)
> +{
> +	struct tegra_vde_cache_entry *entry, *tmp;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	do {
> +		list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
> +			if (!cancel_delayed_work(&entry->dwork))
> +				continue;
> +
> +			tegra_vde_release_entry(entry);
> +		}
> +
> +		mutex_unlock(&vde->map_lock);
> +		schedule();
> +		mutex_lock(&vde->map_lock);
> +	} while (!list_empty(&vde->map_list));
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
> index 295c3d7cccd3..6d635332e0ec 100644
> --- a/drivers/staging/media/tegra-vde/iommu.c
> +++ b/drivers/staging/media/tegra-vde/iommu.c
> @@ -19,7 +19,6 @@
>  int tegra_vde_iommu_map(struct tegra_vde *vde,
>  			struct sg_table *sgt,
>  			struct iova **iovap,
> -			dma_addr_t *addrp,
>  			size_t size)
>  {
>  	struct iova *iova;
> @@ -45,7 +44,6 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
>  	}
>  
>  	*iovap = iova;
> -	*addrp = addr;
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
> index cbcdbfef072d..3466daddf663 100644
> --- a/drivers/staging/media/tegra-vde/vde.c
> +++ b/drivers/staging/media/tegra-vde/vde.c
> @@ -11,6 +11,7 @@
>  #include <linux/genalloc.h>
>  #include <linux/interrupt.h>
>  #include <linux/iopoll.h>
> +#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> @@ -37,18 +38,10 @@
>  #define BSE_DMA_BUSY		BIT(23)
>  
>  struct video_frame {
> -	struct iova *y_iova;
> -	struct iova *cb_iova;
> -	struct iova *cr_iova;
> -	struct iova *aux_iova;
>  	struct dma_buf_attachment *y_dmabuf_attachment;
>  	struct dma_buf_attachment *cb_dmabuf_attachment;
>  	struct dma_buf_attachment *cr_dmabuf_attachment;
>  	struct dma_buf_attachment *aux_dmabuf_attachment;
> -	struct sg_table *y_sgt;
> -	struct sg_table *cb_sgt;
> -	struct sg_table *cr_sgt;
> -	struct sg_table *aux_sgt;
>  	dma_addr_t y_addr;
>  	dma_addr_t cb_addr;
>  	dma_addr_t cr_addr;
> @@ -494,22 +487,6 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
>  			 vde->sxe, 0x00);
>  }
>  
> -static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
> -					    enum dma_data_direction dma_dir,
> -					    struct dma_buf_attachment *a,
> -					    struct sg_table *sgt,
> -					    struct iova *iova)
> -{
> -	struct dma_buf *dmabuf = a->dmabuf;
> -
> -	if (vde->domain)
> -		tegra_vde_iommu_unmap(vde, iova);
> -
> -	dma_buf_unmap_attachment(a, sgt, dma_dir);
> -	dma_buf_detach(dmabuf, a);
> -	dma_buf_put(dmabuf);
> -}
> -
>  static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  				   int fd,
>  				   unsigned long offset,
> @@ -517,15 +494,11 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  				   size_t align_size,
>  				   struct dma_buf_attachment **a,
>  				   dma_addr_t *addrp,
> -				   struct sg_table **s,
> -				   struct iova **iovap,
>  				   size_t *size,
>  				   enum dma_data_direction dma_dir)
>  {
>  	struct device *dev = vde->miscdev.parent;
> -	struct dma_buf_attachment *attachment;
>  	struct dma_buf *dmabuf;
> -	struct sg_table *sgt;
>  	int err;
>  
>  	dmabuf = dma_buf_get(fd);
> @@ -546,49 +519,17 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  		return -EINVAL;
>  	}
>  
> -	attachment = dma_buf_attach(dmabuf, dev);
> -	if (IS_ERR(attachment)) {
> -		dev_err(dev, "Failed to attach dmabuf\n");
> -		err = PTR_ERR(attachment);
> +	err = tegra_vde_dmabuf_cache_map(vde, dmabuf, dma_dir, a, addrp);
> +	if (err)
>  		goto err_put;
> -	}
> -
> -	sgt = dma_buf_map_attachment(attachment, dma_dir);
> -	if (IS_ERR(sgt)) {
> -		dev_err(dev, "Failed to get dmabufs sg_table\n");
> -		err = PTR_ERR(sgt);
> -		goto err_detach;
> -	}
> -
> -	if (!vde->domain && sgt->nents > 1) {
> -		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
> -		err = -EINVAL;
> -		goto err_unmap;
> -	}
> -
> -	if (vde->domain) {
> -		err = tegra_vde_iommu_map(vde, sgt, iovap, addrp, dmabuf->size);
> -		if (err) {
> -			dev_err(dev, "IOMMU mapping failed: %d\n", err);
> -			goto err_unmap;
> -		}
> -	} else {
> -		*addrp = sg_dma_address(sgt->sgl);
> -	}
>  
>  	*addrp = *addrp + offset;
> -	*a = attachment;
> -	*s = sgt;
>  
>  	if (size)
>  		*size = dmabuf->size - offset;
>  
>  	return 0;
>  
> -err_unmap:
> -	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
> -err_detach:
> -	dma_buf_detach(dmabuf, attachment);
>  err_put:
>  	dma_buf_put(dmabuf);
>  
> @@ -608,8 +549,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->y_offset, lsize, SZ_256,
>  				      &frame->y_dmabuf_attachment,
>  				      &frame->y_addr,
> -				      &frame->y_sgt,
> -				      &frame->y_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		return err;
> @@ -618,8 +557,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->cb_offset, csize, SZ_256,
>  				      &frame->cb_dmabuf_attachment,
>  				      &frame->cb_addr,
> -				      &frame->cb_sgt,
> -				      &frame->cb_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_y;
> @@ -628,8 +565,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->cr_offset, csize, SZ_256,
>  				      &frame->cr_dmabuf_attachment,
>  				      &frame->cr_addr,
> -				      &frame->cr_sgt,
> -				      &frame->cr_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cb;
> @@ -643,8 +578,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->aux_offset, csize, SZ_256,
>  				      &frame->aux_dmabuf_attachment,
>  				      &frame->aux_addr,
> -				      &frame->aux_sgt,
> -				      &frame->aux_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cr;
> @@ -652,20 +585,11 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  	return 0;
>  
>  err_release_cr:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cr_dmabuf_attachment,
> -					frame->cr_sgt,
> -					frame->cr_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, true);
>  err_release_cb:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cb_dmabuf_attachment,
> -					frame->cb_sgt,
> -					frame->cb_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, true);
>  err_release_y:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->y_dmabuf_attachment,
> -					frame->y_sgt,
> -					frame->y_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, true);
>  
>  	return err;
>  }
> @@ -673,28 +597,16 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  static void tegra_vde_release_frame_dmabufs(struct tegra_vde *vde,
>  					    struct video_frame *frame,
>  					    enum dma_data_direction dma_dir,
> -					    bool baseline_profile)
> +					    bool baseline_profile,
> +					    bool release)
>  {
>  	if (!baseline_profile)
> -		tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -						frame->aux_dmabuf_attachment,
> -						frame->aux_sgt,
> -						frame->aux_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cr_dmabuf_attachment,
> -					frame->cr_sgt,
> -					frame->cr_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cb_dmabuf_attachment,
> -					frame->cb_sgt,
> -					frame->cb_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->y_dmabuf_attachment,
> -					frame->y_sgt,
> -					frame->y_iova);
> +		tegra_vde_dmabuf_cache_unmap(vde, frame->aux_dmabuf_attachment,
> +					     release);
> +
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, release);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, release);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, release);
>  }
>  
>  static int tegra_vde_validate_frame(struct device *dev,
> @@ -786,8 +698,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	struct tegra_vde_h264_frame __user *frames_user;
>  	struct video_frame *dpb_frames;
>  	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
> -	struct sg_table *bitstream_sgt;
> -	struct iova *bitstream_iova;
>  	enum dma_data_direction dma_dir;
>  	dma_addr_t bitstream_data_addr;
>  	dma_addr_t bsev_ptr;
> @@ -812,8 +722,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  				      SZ_16K, SZ_16K,
>  				      &bitstream_data_dmabuf_attachment,
>  				      &bitstream_data_addr,
> -				      &bitstream_sgt,
> -				      &bitstream_iova,
>  				      &bitstream_data_size,
>  				      DMA_TO_DEVICE);
>  	if (ret)
> @@ -944,7 +852,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>  
>  		tegra_vde_release_frame_dmabufs(vde, &dpb_frames[i], dma_dir,
> -						ctx.baseline_profile);
> +						ctx.baseline_profile, ret != 0);
>  	}
>  
>  free_dpb_frames:
> @@ -954,10 +862,8 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	kfree(frames);
>  
>  release_bitstream_dmabuf:
> -	tegra_vde_detach_and_put_dmabuf(vde, DMA_TO_DEVICE,
> -					bitstream_data_dmabuf_attachment,
> -					bitstream_sgt,
> -					bitstream_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, bitstream_data_dmabuf_attachment,
> +				     ret != 0);
>  
>  	return ret;
>  }
> @@ -979,9 +885,21 @@ static long tegra_vde_unlocked_ioctl(struct file *filp,
>  	return -ENOTTY;
>  }
>  
> +static int tegra_vde_release_file(struct inode *inode, struct file *filp)
> +{
> +	struct miscdevice *miscdev = filp->private_data;
> +	struct tegra_vde *vde = container_of(miscdev, struct tegra_vde,
> +					     miscdev);
> +
> +	tegra_vde_dmabuf_cache_unmap_sync(vde);
> +
> +	return 0;
> +}
> +
>  static const struct file_operations tegra_vde_fops = {
>  	.owner		= THIS_MODULE,
>  	.unlocked_ioctl	= tegra_vde_unlocked_ioctl,
> +	.release	= tegra_vde_release_file,
>  };
>  
>  static irqreturn_t tegra_vde_isr(int irq, void *data)
> @@ -1159,6 +1077,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	INIT_LIST_HEAD(&vde->map_list);
> +	mutex_init(&vde->map_lock);
>  	mutex_init(&vde->lock);
>  	init_completion(&vde->decode_completion);
>  
> @@ -1221,6 +1141,7 @@ static int tegra_vde_remove(struct platform_device *pdev)
>  
>  	misc_deregister(&vde->miscdev);
>  
> +	tegra_vde_dmabuf_cache_unmap_all(vde);
>  	tegra_vde_iommu_deinit(vde);
>  
>  	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
> diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
> index 37414b7fdee1..e138878e8e14 100644
> --- a/drivers/staging/media/tegra-vde/vde.h
> +++ b/drivers/staging/media/tegra-vde/vde.h
> @@ -9,16 +9,20 @@
>  #define TEGRA_VDE_H
>  
>  #include <linux/completion.h>
> +#include <linux/dma-direction.h>
> +#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/mutex.h>
>  #include <linux/types.h>
>  #include <linux/iova.h>
>  
>  struct clk;
> +struct dma_buf;
>  struct gen_pool;
>  struct iommu_group;
>  struct iommu_domain;
>  struct reset_control;
> +struct dma_buf_attachment;
>  
>  struct tegra_vde {
>  	void __iomem *sxe;
> @@ -31,6 +35,8 @@ struct tegra_vde {
>  	void __iomem *vdma;
>  	void __iomem *frameid;
>  	struct mutex lock;
> +	struct mutex map_lock;
> +	struct list_head map_list;
>  	struct miscdevice miscdev;
>  	struct reset_control *rst;
>  	struct reset_control *rst_mc;
> @@ -49,10 +55,20 @@ void tegra_vde_iommu_deinit(struct tegra_vde *vde);
>  int tegra_vde_iommu_map(struct tegra_vde *vde,
>  			struct sg_table *sgt,
>  			struct iova **iovap,
> -			dma_addr_t *addrp,
>  			size_t size);
>  void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova);
>  
> +int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
> +			       struct dma_buf *dmabuf,
> +			       enum dma_data_direction dma_dir,
> +			       struct dma_buf_attachment **ap,
> +			       dma_addr_t *addrp);
> +void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
> +				  struct dma_buf_attachment *a,
> +				  bool release);
> +void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde);
> +void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde);
> +
>  static __maybe_unused char const *
>  tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
>  {
> 

Re: [PATCH v1 4/4] staging: media: tegra-vde: Defer dmabuf's unmapping

[Hans Verkuil]

On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
> Frequent IOMMU remappings take about 50% of CPU usage because there is
> quite a lot to remap. Defer dmabuf's unmapping by 5 seconds in order to
> mitigate the mapping overhead which goes away completely and driver works
> as fast as in a case of a disabled IOMMU. The case of a disabled IOMMU
> should also benefit a tad from the caching since CPU cache maintenance
> that happens on dmabuf's attaching takes some resources.
> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/staging/media/tegra-vde/Makefile      |   2 +-
>  .../staging/media/tegra-vde/dmabuf-cache.c    | 223 ++++++++++++++++++
>  drivers/staging/media/tegra-vde/iommu.c       |   2 -
>  drivers/staging/media/tegra-vde/vde.c         | 143 +++--------
>  drivers/staging/media/tegra-vde/vde.h         |  18 +-
>  5 files changed, 273 insertions(+), 115 deletions(-)
>  create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c
> 
> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
> index c11867e28233..2827f7601de8 100644
> --- a/drivers/staging/media/tegra-vde/Makefile
> +++ b/drivers/staging/media/tegra-vde/Makefile
> @@ -1,3 +1,3 @@
>  # SPDX-License-Identifier: GPL-2.0
> -tegra-vde-y := vde.o iommu.o
> +tegra-vde-y := vde.o iommu.o dmabuf-cache.o
>  obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
> diff --git a/drivers/staging/media/tegra-vde/dmabuf-cache.c b/drivers/staging/media/tegra-vde/dmabuf-cache.c
> new file mode 100644
> index 000000000000..fcde8d1c37e7
> --- /dev/null
> +++ b/drivers/staging/media/tegra-vde/dmabuf-cache.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * NVIDIA Tegra Video decoder driver
> + *
> + * Copyright (C) 2016-2019 GRATE-DRIVER project
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/iova.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +
> +#include "vde.h"
> +
> +struct tegra_vde_cache_entry {
> +	enum dma_data_direction dma_dir;
> +	struct dma_buf_attachment *a;
> +	struct delayed_work dwork;
> +	struct tegra_vde *vde;
> +	struct list_head list;
> +	struct sg_table *sgt;
> +	struct iova *iova;
> +	unsigned int refcnt;
> +};
> +
> +static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry)
> +{
> +	struct dma_buf *dmabuf = entry->a->dmabuf;
> +
> +	WARN_ON_ONCE(entry->refcnt);
> +
> +	if (entry->vde->domain)
> +		tegra_vde_iommu_unmap(entry->vde, entry->iova);
> +
> +	dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
> +	dma_buf_detach(dmabuf, entry->a);
> +	dma_buf_put(dmabuf);
> +
> +	list_del(&entry->list);
> +	kfree(entry);
> +}
> +
> +static void tegra_vde_delayed_unmap(struct work_struct *work)
> +{
> +	struct tegra_vde_cache_entry *entry;
> +
> +	entry = container_of(work, struct tegra_vde_cache_entry,
> +			     dwork.work);
> +
> +	mutex_lock(&entry->vde->map_lock);
> +	tegra_vde_release_entry(entry);
> +	mutex_unlock(&entry->vde->map_lock);

>From smatch:

drivers/staging/media/tegra-vde/dmabuf-cache.c:55 tegra_vde_delayed_unmap() error: dereferencing freed memory 'entry'

> +}
> +
> +int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
> +			       struct dma_buf *dmabuf,
> +			       enum dma_data_direction dma_dir,
> +			       struct dma_buf_attachment **ap,
> +			       dma_addr_t *addrp)
> +{
> +	struct device *dev = vde->miscdev.parent;
> +	struct tegra_vde_cache_entry *entry;
> +	struct dma_buf_attachment *attachment;
> +	struct sg_table *sgt;
> +	struct iova *iova;
> +	int err;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry(entry, &vde->map_list, list) {
> +		if (entry->a->dmabuf != dmabuf)
> +			continue;
> +
> +		if (!cancel_delayed_work(&entry->dwork))
> +			continue;
> +
> +		if (entry->dma_dir != dma_dir)
> +			entry->dma_dir = DMA_BIDIRECTIONAL;
> +
> +		dma_buf_put(dmabuf);
> +
> +		if (vde->domain)
> +			*addrp = iova_dma_addr(&vde->iova, entry->iova);
> +		else
> +			*addrp = sg_dma_address(entry->sgt->sgl);
> +
> +		goto ref;
> +	}
> +
> +	attachment = dma_buf_attach(dmabuf, dev);
> +	if (IS_ERR(attachment)) {
> +		dev_err(dev, "Failed to attach dmabuf\n");
> +		err = PTR_ERR(attachment);
> +		goto err_unlock;
> +	}
> +
> +	sgt = dma_buf_map_attachment(attachment, dma_dir);
> +	if (IS_ERR(sgt)) {
> +		dev_err(dev, "Failed to get dmabufs sg_table\n");
> +		err = PTR_ERR(sgt);
> +		goto err_detach;
> +	}
> +
> +	if (!vde->domain && sgt->nents > 1) {
> +		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
> +		err = -EINVAL;
> +		goto err_unmap;
> +	}
> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry) {
> +		err = -ENOMEM;
> +		goto err_unmap;
> +	}
> +
> +	if (vde->domain) {
> +		err = tegra_vde_iommu_map(vde, sgt, &iova, dmabuf->size);
> +		if (err)
> +			goto err_free;
> +
> +		*addrp = iova_dma_addr(&vde->iova, iova);
> +	} else {
> +		*addrp = sg_dma_address(sgt->sgl);
> +	}
> +
> +	INIT_DELAYED_WORK(&entry->dwork, tegra_vde_delayed_unmap);
> +	list_add(&entry->list, &vde->map_list);
> +
> +	entry->dma_dir = dma_dir;
> +	entry->iova = iova;

>From smatch:

drivers/staging/media/tegra-vde/dmabuf-cache.c:133 tegra_vde_dmabuf_cache_map() error: uninitialized symbol 'iova'.

Regards,

	Hans

> +	entry->vde = vde;
> +	entry->sgt = sgt;
> +	entry->a = attachment;
> +ref:
> +	entry->refcnt++;
> +
> +	*ap = entry->a;
> +
> +	mutex_unlock(&vde->map_lock);
> +
> +	return 0;
> +
> +err_free:
> +	kfree(entry);
> +err_unmap:
> +	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
> +err_detach:
> +	dma_buf_detach(dmabuf, attachment);
> +err_unlock:
> +	mutex_unlock(&vde->map_lock);
> +
> +	return err;
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
> +				  struct dma_buf_attachment *a,
> +				  bool release)
> +{
> +	struct tegra_vde_cache_entry *entry;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry(entry, &vde->map_list, list) {
> +		if (entry->a != a)
> +			continue;
> +
> +		WARN_ON_ONCE(!entry->refcnt);
> +
> +		if (--entry->refcnt == 0) {
> +			if (release)
> +				tegra_vde_release_entry(entry);
> +			else
> +				schedule_delayed_work(&entry->dwork, 5 * HZ);
> +		}
> +		break;
> +	}
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde)
> +{
> +	struct tegra_vde_cache_entry *entry, *tmp;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
> +		if (entry->refcnt)
> +			continue;
> +
> +		if (!cancel_delayed_work(&entry->dwork))
> +			continue;
> +
> +		tegra_vde_release_entry(entry);
> +	}
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> +
> +void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde)
> +{
> +	struct tegra_vde_cache_entry *entry, *tmp;
> +
> +	mutex_lock(&vde->map_lock);
> +
> +	do {
> +		list_for_each_entry_safe(entry, tmp, &vde->map_list, list) {
> +			if (!cancel_delayed_work(&entry->dwork))
> +				continue;
> +
> +			tegra_vde_release_entry(entry);
> +		}
> +
> +		mutex_unlock(&vde->map_lock);
> +		schedule();
> +		mutex_lock(&vde->map_lock);
> +	} while (!list_empty(&vde->map_list));
> +
> +	mutex_unlock(&vde->map_lock);
> +}
> diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
> index 295c3d7cccd3..6d635332e0ec 100644
> --- a/drivers/staging/media/tegra-vde/iommu.c
> +++ b/drivers/staging/media/tegra-vde/iommu.c
> @@ -19,7 +19,6 @@
>  int tegra_vde_iommu_map(struct tegra_vde *vde,
>  			struct sg_table *sgt,
>  			struct iova **iovap,
> -			dma_addr_t *addrp,
>  			size_t size)
>  {
>  	struct iova *iova;
> @@ -45,7 +44,6 @@ int tegra_vde_iommu_map(struct tegra_vde *vde,
>  	}
>  
>  	*iovap = iova;
> -	*addrp = addr;
>  
>  	return 0;
>  }
> diff --git a/drivers/staging/media/tegra-vde/vde.c b/drivers/staging/media/tegra-vde/vde.c
> index cbcdbfef072d..3466daddf663 100644
> --- a/drivers/staging/media/tegra-vde/vde.c
> +++ b/drivers/staging/media/tegra-vde/vde.c
> @@ -11,6 +11,7 @@
>  #include <linux/genalloc.h>
>  #include <linux/interrupt.h>
>  #include <linux/iopoll.h>
> +#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> @@ -37,18 +38,10 @@
>  #define BSE_DMA_BUSY		BIT(23)
>  
>  struct video_frame {
> -	struct iova *y_iova;
> -	struct iova *cb_iova;
> -	struct iova *cr_iova;
> -	struct iova *aux_iova;
>  	struct dma_buf_attachment *y_dmabuf_attachment;
>  	struct dma_buf_attachment *cb_dmabuf_attachment;
>  	struct dma_buf_attachment *cr_dmabuf_attachment;
>  	struct dma_buf_attachment *aux_dmabuf_attachment;
> -	struct sg_table *y_sgt;
> -	struct sg_table *cb_sgt;
> -	struct sg_table *cr_sgt;
> -	struct sg_table *aux_sgt;
>  	dma_addr_t y_addr;
>  	dma_addr_t cb_addr;
>  	dma_addr_t cr_addr;
> @@ -494,22 +487,6 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
>  			 vde->sxe, 0x00);
>  }
>  
> -static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
> -					    enum dma_data_direction dma_dir,
> -					    struct dma_buf_attachment *a,
> -					    struct sg_table *sgt,
> -					    struct iova *iova)
> -{
> -	struct dma_buf *dmabuf = a->dmabuf;
> -
> -	if (vde->domain)
> -		tegra_vde_iommu_unmap(vde, iova);
> -
> -	dma_buf_unmap_attachment(a, sgt, dma_dir);
> -	dma_buf_detach(dmabuf, a);
> -	dma_buf_put(dmabuf);
> -}
> -
>  static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  				   int fd,
>  				   unsigned long offset,
> @@ -517,15 +494,11 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  				   size_t align_size,
>  				   struct dma_buf_attachment **a,
>  				   dma_addr_t *addrp,
> -				   struct sg_table **s,
> -				   struct iova **iovap,
>  				   size_t *size,
>  				   enum dma_data_direction dma_dir)
>  {
>  	struct device *dev = vde->miscdev.parent;
> -	struct dma_buf_attachment *attachment;
>  	struct dma_buf *dmabuf;
> -	struct sg_table *sgt;
>  	int err;
>  
>  	dmabuf = dma_buf_get(fd);
> @@ -546,49 +519,17 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
>  		return -EINVAL;
>  	}
>  
> -	attachment = dma_buf_attach(dmabuf, dev);
> -	if (IS_ERR(attachment)) {
> -		dev_err(dev, "Failed to attach dmabuf\n");
> -		err = PTR_ERR(attachment);
> +	err = tegra_vde_dmabuf_cache_map(vde, dmabuf, dma_dir, a, addrp);
> +	if (err)
>  		goto err_put;
> -	}
> -
> -	sgt = dma_buf_map_attachment(attachment, dma_dir);
> -	if (IS_ERR(sgt)) {
> -		dev_err(dev, "Failed to get dmabufs sg_table\n");
> -		err = PTR_ERR(sgt);
> -		goto err_detach;
> -	}
> -
> -	if (!vde->domain && sgt->nents > 1) {
> -		dev_err(dev, "Sparse DMA region is unsupported, please enable IOMMU\n");
> -		err = -EINVAL;
> -		goto err_unmap;
> -	}
> -
> -	if (vde->domain) {
> -		err = tegra_vde_iommu_map(vde, sgt, iovap, addrp, dmabuf->size);
> -		if (err) {
> -			dev_err(dev, "IOMMU mapping failed: %d\n", err);
> -			goto err_unmap;
> -		}
> -	} else {
> -		*addrp = sg_dma_address(sgt->sgl);
> -	}
>  
>  	*addrp = *addrp + offset;
> -	*a = attachment;
> -	*s = sgt;
>  
>  	if (size)
>  		*size = dmabuf->size - offset;
>  
>  	return 0;
>  
> -err_unmap:
> -	dma_buf_unmap_attachment(attachment, sgt, dma_dir);
> -err_detach:
> -	dma_buf_detach(dmabuf, attachment);
>  err_put:
>  	dma_buf_put(dmabuf);
>  
> @@ -608,8 +549,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->y_offset, lsize, SZ_256,
>  				      &frame->y_dmabuf_attachment,
>  				      &frame->y_addr,
> -				      &frame->y_sgt,
> -				      &frame->y_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		return err;
> @@ -618,8 +557,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->cb_offset, csize, SZ_256,
>  				      &frame->cb_dmabuf_attachment,
>  				      &frame->cb_addr,
> -				      &frame->cb_sgt,
> -				      &frame->cb_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_y;
> @@ -628,8 +565,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->cr_offset, csize, SZ_256,
>  				      &frame->cr_dmabuf_attachment,
>  				      &frame->cr_addr,
> -				      &frame->cr_sgt,
> -				      &frame->cr_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cb;
> @@ -643,8 +578,6 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  				      src->aux_offset, csize, SZ_256,
>  				      &frame->aux_dmabuf_attachment,
>  				      &frame->aux_addr,
> -				      &frame->aux_sgt,
> -				      &frame->aux_iova,
>  				      NULL, dma_dir);
>  	if (err)
>  		goto err_release_cr;
> @@ -652,20 +585,11 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  	return 0;
>  
>  err_release_cr:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cr_dmabuf_attachment,
> -					frame->cr_sgt,
> -					frame->cr_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, true);
>  err_release_cb:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cb_dmabuf_attachment,
> -					frame->cb_sgt,
> -					frame->cb_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, true);
>  err_release_y:
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->y_dmabuf_attachment,
> -					frame->y_sgt,
> -					frame->y_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, true);
>  
>  	return err;
>  }
> @@ -673,28 +597,16 @@ static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
>  static void tegra_vde_release_frame_dmabufs(struct tegra_vde *vde,
>  					    struct video_frame *frame,
>  					    enum dma_data_direction dma_dir,
> -					    bool baseline_profile)
> +					    bool baseline_profile,
> +					    bool release)
>  {
>  	if (!baseline_profile)
> -		tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -						frame->aux_dmabuf_attachment,
> -						frame->aux_sgt,
> -						frame->aux_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cr_dmabuf_attachment,
> -					frame->cr_sgt,
> -					frame->cr_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->cb_dmabuf_attachment,
> -					frame->cb_sgt,
> -					frame->cb_iova);
> -
> -	tegra_vde_detach_and_put_dmabuf(vde, dma_dir,
> -					frame->y_dmabuf_attachment,
> -					frame->y_sgt,
> -					frame->y_iova);
> +		tegra_vde_dmabuf_cache_unmap(vde, frame->aux_dmabuf_attachment,
> +					     release);
> +
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cr_dmabuf_attachment, release);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->cb_dmabuf_attachment, release);
> +	tegra_vde_dmabuf_cache_unmap(vde, frame->y_dmabuf_attachment, release);
>  }
>  
>  static int tegra_vde_validate_frame(struct device *dev,
> @@ -786,8 +698,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	struct tegra_vde_h264_frame __user *frames_user;
>  	struct video_frame *dpb_frames;
>  	struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
> -	struct sg_table *bitstream_sgt;
> -	struct iova *bitstream_iova;
>  	enum dma_data_direction dma_dir;
>  	dma_addr_t bitstream_data_addr;
>  	dma_addr_t bsev_ptr;
> @@ -812,8 +722,6 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  				      SZ_16K, SZ_16K,
>  				      &bitstream_data_dmabuf_attachment,
>  				      &bitstream_data_addr,
> -				      &bitstream_sgt,
> -				      &bitstream_iova,
>  				      &bitstream_data_size,
>  				      DMA_TO_DEVICE);
>  	if (ret)
> @@ -944,7 +852,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  		dma_dir = (i == 0) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
>  
>  		tegra_vde_release_frame_dmabufs(vde, &dpb_frames[i], dma_dir,
> -						ctx.baseline_profile);
> +						ctx.baseline_profile, ret != 0);
>  	}
>  
>  free_dpb_frames:
> @@ -954,10 +862,8 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde,
>  	kfree(frames);
>  
>  release_bitstream_dmabuf:
> -	tegra_vde_detach_and_put_dmabuf(vde, DMA_TO_DEVICE,
> -					bitstream_data_dmabuf_attachment,
> -					bitstream_sgt,
> -					bitstream_iova);
> +	tegra_vde_dmabuf_cache_unmap(vde, bitstream_data_dmabuf_attachment,
> +				     ret != 0);
>  
>  	return ret;
>  }
> @@ -979,9 +885,21 @@ static long tegra_vde_unlocked_ioctl(struct file *filp,
>  	return -ENOTTY;
>  }
>  
> +static int tegra_vde_release_file(struct inode *inode, struct file *filp)
> +{
> +	struct miscdevice *miscdev = filp->private_data;
> +	struct tegra_vde *vde = container_of(miscdev, struct tegra_vde,
> +					     miscdev);
> +
> +	tegra_vde_dmabuf_cache_unmap_sync(vde);
> +
> +	return 0;
> +}
> +
>  static const struct file_operations tegra_vde_fops = {
>  	.owner		= THIS_MODULE,
>  	.unlocked_ioctl	= tegra_vde_unlocked_ioctl,
> +	.release	= tegra_vde_release_file,
>  };
>  
>  static irqreturn_t tegra_vde_isr(int irq, void *data)
> @@ -1159,6 +1077,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	INIT_LIST_HEAD(&vde->map_list);
> +	mutex_init(&vde->map_lock);
>  	mutex_init(&vde->lock);
>  	init_completion(&vde->decode_completion);
>  
> @@ -1221,6 +1141,7 @@ static int tegra_vde_remove(struct platform_device *pdev)
>  
>  	misc_deregister(&vde->miscdev);
>  
> +	tegra_vde_dmabuf_cache_unmap_all(vde);
>  	tegra_vde_iommu_deinit(vde);
>  
>  	gen_pool_free(vde->iram_pool, (unsigned long)vde->iram,
> diff --git a/drivers/staging/media/tegra-vde/vde.h b/drivers/staging/media/tegra-vde/vde.h
> index 37414b7fdee1..e138878e8e14 100644
> --- a/drivers/staging/media/tegra-vde/vde.h
> +++ b/drivers/staging/media/tegra-vde/vde.h
> @@ -9,16 +9,20 @@
>  #define TEGRA_VDE_H
>  
>  #include <linux/completion.h>
> +#include <linux/dma-direction.h>
> +#include <linux/list.h>
>  #include <linux/miscdevice.h>
>  #include <linux/mutex.h>
>  #include <linux/types.h>
>  #include <linux/iova.h>
>  
>  struct clk;
> +struct dma_buf;
>  struct gen_pool;
>  struct iommu_group;
>  struct iommu_domain;
>  struct reset_control;
> +struct dma_buf_attachment;
>  
>  struct tegra_vde {
>  	void __iomem *sxe;
> @@ -31,6 +35,8 @@ struct tegra_vde {
>  	void __iomem *vdma;
>  	void __iomem *frameid;
>  	struct mutex lock;
> +	struct mutex map_lock;
> +	struct list_head map_list;
>  	struct miscdevice miscdev;
>  	struct reset_control *rst;
>  	struct reset_control *rst_mc;
> @@ -49,10 +55,20 @@ void tegra_vde_iommu_deinit(struct tegra_vde *vde);
>  int tegra_vde_iommu_map(struct tegra_vde *vde,
>  			struct sg_table *sgt,
>  			struct iova **iovap,
> -			dma_addr_t *addrp,
>  			size_t size);
>  void tegra_vde_iommu_unmap(struct tegra_vde *vde, struct iova *iova);
>  
> +int tegra_vde_dmabuf_cache_map(struct tegra_vde *vde,
> +			       struct dma_buf *dmabuf,
> +			       enum dma_data_direction dma_dir,
> +			       struct dma_buf_attachment **ap,
> +			       dma_addr_t *addrp);
> +void tegra_vde_dmabuf_cache_unmap(struct tegra_vde *vde,
> +				  struct dma_buf_attachment *a,
> +				  bool release);
> +void tegra_vde_dmabuf_cache_unmap_sync(struct tegra_vde *vde);
> +void tegra_vde_dmabuf_cache_unmap_all(struct tegra_vde *vde);
> +
>  static __maybe_unused char const *
>  tegra_vde_reg_base_name(struct tegra_vde *vde, void __iomem *base)
>  {
> 

Re: [PATCH v1 4/4] staging: media: tegra-vde: Defer dmabuf's unmapping

[Dmitry Osipenko]

17.06.2019 16:33, Hans Verkuil пишет:
> On 6/2/19 11:37 PM, Dmitry Osipenko wrote:
>> Frequent IOMMU remappings take about 50% of CPU usage because there is
>> quite a lot to remap. Defer dmabuf's unmapping by 5 seconds in order to
>> mitigate the mapping overhead which goes away completely and driver works
>> as fast as in a case of a disabled IOMMU. The case of a disabled IOMMU
>> should also benefit a tad from the caching since CPU cache maintenance
>> that happens on dmabuf's attaching takes some resources.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>>  drivers/staging/media/tegra-vde/Makefile      |   2 +-
>>  .../staging/media/tegra-vde/dmabuf-cache.c    | 223 ++++++++++++++++++
>>  drivers/staging/media/tegra-vde/iommu.c       |   2 -
>>  drivers/staging/media/tegra-vde/vde.c         | 143 +++--------
>>  drivers/staging/media/tegra-vde/vde.h         |  18 +-
>>  5 files changed, 273 insertions(+), 115 deletions(-)
>>  create mode 100644 drivers/staging/media/tegra-vde/dmabuf-cache.c
>>
>> diff --git a/drivers/staging/media/tegra-vde/Makefile b/drivers/staging/media/tegra-vde/Makefile
>> index c11867e28233..2827f7601de8 100644
>> --- a/drivers/staging/media/tegra-vde/Makefile
>> +++ b/drivers/staging/media/tegra-vde/Makefile
>> @@ -1,3 +1,3 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> -tegra-vde-y := vde.o iommu.o
>> +tegra-vde-y := vde.o iommu.o dmabuf-cache.o
>>  obj-$(CONFIG_TEGRA_VDE)	+= tegra-vde.o
>> diff --git a/drivers/staging/media/tegra-vde/dmabuf-cache.c b/drivers/staging/media/tegra-vde/dmabuf-cache.c
>> new file mode 100644
>> index 000000000000..fcde8d1c37e7
>> --- /dev/null
>> +++ b/drivers/staging/media/tegra-vde/dmabuf-cache.c
>> @@ -0,0 +1,223 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * NVIDIA Tegra Video decoder driver
>> + *
>> + * Copyright (C) 2016-2019 GRATE-DRIVER project
>> + */
>> +
>> +#include <linux/dma-buf.h>
>> +#include <linux/iova.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/workqueue.h>
>> +
>> +#include "vde.h"
>> +
>> +struct tegra_vde_cache_entry {
>> +	enum dma_data_direction dma_dir;
>> +	struct dma_buf_attachment *a;
>> +	struct delayed_work dwork;
>> +	struct tegra_vde *vde;
>> +	struct list_head list;
>> +	struct sg_table *sgt;
>> +	struct iova *iova;
>> +	unsigned int refcnt;
>> +};
>> +
>> +static void tegra_vde_release_entry(struct tegra_vde_cache_entry *entry)
>> +{
>> +	struct dma_buf *dmabuf = entry->a->dmabuf;
>> +
>> +	WARN_ON_ONCE(entry->refcnt);
>> +
>> +	if (entry->vde->domain)
>> +		tegra_vde_iommu_unmap(entry->vde, entry->iova);
>> +
>> +	dma_buf_unmap_attachment(entry->a, entry->sgt, entry->dma_dir);
>> +	dma_buf_detach(dmabuf, entry->a);
>> +	dma_buf_put(dmabuf);
>> +
>> +	list_del(&entry->list);
>> +	kfree(entry);
>> +}
>> +
>> +static void tegra_vde_delayed_unmap(struct work_struct *work)
>> +{
>> +	struct tegra_vde_cache_entry *entry;
>> +
>> +	entry = container_of(work, struct tegra_vde_cache_entry,
>> +			     dwork.work);
>> +
>> +	mutex_lock(&entry->vde->map_lock);
>> +	tegra_vde_release_entry(entry);
>> +	mutex_unlock(&entry->vde->map_lock);
> 
> From smatch:
> 
> drivers/staging/media/tegra-vde/dmabuf-cache.c:55 tegra_vde_delayed_unmap() error: dereferencing freed memory 'entry'

That's a very good catch, thanks you very much! I'm keep forgetting about smatch, it's
a useful tool. And unfortunately I can't KASAN the driver because ARM32 doesn't
support KASAN in upstream and Xorg hangs with the unofficial patch that adds support
for the KASAN.

[snip]

>> +	entry->dma_dir = dma_dir;
>> +	entry->iova = iova;
> 
> From smatch:
> 
> drivers/staging/media/tegra-vde/dmabuf-cache.c:133 tegra_vde_dmabuf_cache_map() error: uninitialized symbol 'iova'.

This is fine, but indeed won't hurt to explicitly initialize to NULL.

Thanks again!