public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks
@ 2025-09-11 15:57 Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 1/7] dt-bindings: vendor-prefixes: Add Verisilicon Benjamin Gaignard
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 4067 bytes --]

Hi all,

This patch series adds support for the Verisilicon IOMMU, which is found in front
of hardware encoder and decoder blocks in several SoCs using Verisilicon IP. 
A first implementation of this IOMMU is available on the Rockchip RK3588 SoC.

Rockchip provides a driver for this hardware in their 6.1 kernel branch:
https://github.com/rockchip-linux/kernel/blob/develop-6.1/drivers/iommu/rockchip-iommu-av1d.c

This series includes:
- a new binding for the Verisilicon IOMMU
- a driver implementation
- DT updates for RK3588

The driver was forward-ported from Rockchip’s 6.1 implementation, 
the prefix was renamed to vsi for generality, and several fixes were applied.

AV1 decoding was tested using the stateless VPU driver and Fluster.
The test results show a score of 205/239, which confirms that no regressions
were introduced by this series.

Feedback and testing welcome.

changes in version 9:
- removing blanks lines.

changes in version 8:
- Add myself in MAINTAINERS file.
- Add API to restore VSI iommu context from decoder driver
- Fix reported checkpatch issues: add comment in pinlock_t declaration
  and remove blank line.
- Include board name in defconfig patch commit message

changes in version 7:
- fix locking issues.
- add a patch in AV1 video decoder to manage per context iommu domain.
- fix compilation issues when build as module.
- remove useless "rockchip,rk3588-av1-iommu" compatible in driver code.

changes in version 6:
- rework lock schema in vsi_iommu_attach_device() so
  it protected against concurrent invalidation.
- flush the cache after changing of domain.

changes in version 5:
- change locking schema to use 2 spin_locks: one to protect vsi_domain
  data and one to protect vsi_iommu structure.
- make suspend/resume more robust by calling disable/enable function.
- rebased on top of v6.16-rc5

changes in version 4:
- rename and reorder compatibles fields.
- Kconfig dependencies
- Fix the remarks done by Jason and Robin: locking, clocks, macros
  probing, pm_runtime, atomic allocation.

changes in version 3:
- Change compatible to "rockchip,rk3588-iommu-1.2"
- Fix compatible in .yaml
- Update DT and driver to use "rockchip,rk3588-iommu-1.2" compatible
- Set CONFIG_VSI_IOMMU as module in defconfig
- Create an identity domain for the driver
- Fix double flush issue
- Rework attach/detach logic
- Simplify xlate function
- Discover iommu device like done in ARM driver
- Remove ARM_DMA_USE_IOMMU from Kconfig

changes in version 2:
- Add a compatible "rockchip,rk3588-av1-iommu"
- Fix clock-names in binding 
- Remove "vsi_mmu" label in binding example.
- Rework driver probe function
- Remove double flush
- Rework driver internal structures and avoid allocate
  in xlate function.
- Do not touch to VPU driver anymore (path removed)
- Add a patch to enable the driver in arm64 defconfig

Benjamin Gaignard (7):
  dt-bindings: vendor-prefixes: Add Verisilicon
  dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU
  iommu: Add verisilicon IOMMU driver
  MAINTAINERS: Add entry for Verisilicon IOMMU driver
  media: verisilicon: AV1: Restore IOMMU context before decoding a frame
  arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588
  arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588

 .../bindings/iommu/verisilicon,iommu.yaml     |  71 ++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   8 +
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi |  11 +
 arch/arm64/configs/defconfig                  |   1 +
 drivers/iommu/Kconfig                         |  11 +
 drivers/iommu/Makefile                        |   1 +
 drivers/iommu/vsi-iommu.c                     | 808 ++++++++++++++++++
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c  |  15 +
 include/linux/vsi-iommu.h                     |  21 +
 10 files changed, 949 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
 create mode 100644 drivers/iommu/vsi-iommu.c
 create mode 100644 include/linux/vsi-iommu.h

-- 
2.43.0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v9 1/7] dt-bindings: vendor-prefixes: Add Verisilicon
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 2/7] dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU Benjamin Gaignard
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard, Conor Dooley

Verisilicon Microelectronics is a company based in Shanghai, China,
developping hardware blocks for SoC.

https://verisilicon.com/

Add their name to the list of vendors.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 9ec8947dfcad..52b3495ff4db 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1656,6 +1656,8 @@ patternProperties:
     description: Variscite Ltd.
   "^vdl,.*":
     description: Van der Laan b.v.
+  "^verisilicon,.*":
+    description: VeriSilicon Microelectronics
   "^vertexcom,.*":
     description: Vertexcom Technologies, Inc.
   "^via,.*":
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 2/7] dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 1/7] dt-bindings: vendor-prefixes: Add Verisilicon Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver Benjamin Gaignard
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard, Conor Dooley

Add a device tree binding for the Verisilicon (VSI) IOMMU.
This IOMMU sits in front of hardware encoder and decoder
blocks on SoCs using Verisilicon IP, such as the Rockchip RK3588.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/iommu/verisilicon,iommu.yaml     | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml

diff --git a/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml b/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
new file mode 100644
index 000000000000..d3ce9e603b61
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/verisilicon,iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Verisilicon IOMMU
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@collabora.com>
+
+description: |+
+  A Versilicon iommu translates io virtual addresses to physical addresses for
+  its associated video decoder.
+
+properties:
+  compatible:
+    items:
+      - const: rockchip,rk3588-av1-iommu
+      - const: verisilicon,iommu-1.2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Core clock
+      - description: Interface clock
+
+  clock-names:
+    items:
+      - const: core
+      - const: iface
+
+  "#iommu-cells":
+    const: 0
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - "#iommu-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    bus {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      iommu@fdca0000 {
+        compatible = "rockchip,rk3588-av1-iommu","verisilicon,iommu-1.2";
+        reg = <0x0 0xfdca0000 0x0 0x600>;
+        interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH 0>;
+        clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+        clock-names = "core", "iface";
+        #iommu-cells = <0>;
+      };
+    };
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 1/7] dt-bindings: vendor-prefixes: Add Verisilicon Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 2/7] dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-12 11:47   ` Jörg Rödel
  2025-12-12  3:31   ` Jianfeng Liu
  2025-09-11 15:57 ` [PATCH v9 4/7] MAINTAINERS: Add entry for Verisilicon " Benjamin Gaignard
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

The Verisilicon IOMMU hardware block can be found in combination
with Verisilicon hardware video codecs (encoders or decoders) on
different SoCs.
Enable it will allow us to use non contiguous memory allocators
for Verisilicon video codecs.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 drivers/iommu/Kconfig     |  11 +
 drivers/iommu/Makefile    |   1 +
 drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
 include/linux/vsi-iommu.h |  21 +
 4 files changed, 841 insertions(+)
 create mode 100644 drivers/iommu/vsi-iommu.c
 create mode 100644 include/linux/vsi-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 70d29b14d851..d3731be630a2 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -383,4 +383,15 @@ config SPRD_IOMMU
 
 	  Say Y here if you want to use the multimedia devices listed above.
 
+config VSI_IOMMU
+	tristate "Verisilicon IOMMU Support"
+	depends on (ARCH_ROCKCHIP && ARM64) || COMPILE_TEST
+	select IOMMU_API
+	help
+	  Support for IOMMUs used by Verisilicon sub-systems like video
+	  decoders or encoder hardware blocks.
+
+	  Say Y here if you want to use this IOMMU in front of these
+	  hardware blocks.
+
 endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 355294fa9033..68aeff31af8b 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_IOMMU_SVA) += iommu-sva.o
 obj-$(CONFIG_IOMMU_IOPF) += io-pgfault.o
 obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
 obj-$(CONFIG_APPLE_DART) += apple-dart.o
+obj-$(CONFIG_VSI_IOMMU) += vsi-iommu.o
diff --git a/drivers/iommu/vsi-iommu.c b/drivers/iommu/vsi-iommu.c
new file mode 100644
index 000000000000..594e1597f405
--- /dev/null
+++ b/drivers/iommu/vsi-iommu.c
@@ -0,0 +1,808 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (C) 2025 Collabora Ltd.
+ *
+ * IOMMU API for Verisilicon
+ *
+ * Module Authors:	Yandong Lin <yandong.lin@rock-chips.com>
+ *			Simon Xue <xxm@rock-chips.com>
+ *			Benjamin Gaignard <benjamin.gaignard@collabora.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/compiler.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/vsi-iommu.h>
+
+#include "iommu-pages.h"
+
+struct vsi_iommu {
+	struct device *dev;
+	void __iomem *regs;
+	struct clk_bulk_data *clocks;
+	int num_clocks;
+	struct iommu_device iommu;
+	struct list_head node; /* entry in vsi_iommu_domain.iommus */
+	struct iommu_domain *domain; /* domain to which iommu is attached */
+	spinlock_t lock; /* lock to protect vsi_iommu fields */
+	int irq;
+};
+
+struct vsi_iommu_domain {
+	struct list_head iommus;
+	struct device *dev;
+	u32 *dt;
+	dma_addr_t dt_dma;
+	struct iommu_domain domain;
+	u64 *pta;
+	dma_addr_t pta_dma;
+	spinlock_t lock; /* lock to protect vsi_iommu_domain fields */
+};
+
+static struct iommu_domain vsi_identity_domain;
+
+#define NUM_DT_ENTRIES	1024
+#define NUM_PT_ENTRIES	1024
+#define PT_SIZE		(NUM_PT_ENTRIES * sizeof(u32))
+
+#define SPAGE_SIZE	BIT(12)
+
+/* vsi iommu regs address */
+#define VSI_MMU_CONFIG1_BASE			0x1ac
+#define VSI_MMU_AHB_EXCEPTION_BASE		0x380
+#define VSI_MMU_AHB_CONTROL_BASE		0x388
+#define VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE	0x38C
+
+/* MMU register offsets */
+#define VSI_MMU_FLUSH_BASE		0x184
+#define VSI_MMU_BIT_FLUSH		BIT(4)
+
+#define VSI_MMU_PAGE_FAULT_ADDR		0x380
+#define VSI_MMU_STATUS_BASE		0x384	/* IRQ status */
+
+#define VSI_MMU_BIT_ENABLE		BIT(0)
+
+#define VSI_MMU_OUT_OF_BOUND		BIT(28)
+/* Irq mask */
+#define VSI_MMU_IRQ_MASK		0x7
+
+#define VSI_DTE_PT_ADDRESS_MASK		0xffffffc0
+#define VSI_DTE_PT_VALID		BIT(0)
+
+#define VSI_PAGE_DESC_LO_MASK		0xfffff000
+#define VSI_PAGE_DESC_HI_MASK		GENMASK_ULL(39, 32)
+#define VSI_PAGE_DESC_HI_SHIFT		(32 - 4)
+
+static inline phys_addr_t vsi_dte_pt_address(u32 dte)
+{
+	return (phys_addr_t)dte & VSI_DTE_PT_ADDRESS_MASK;
+}
+
+static inline u32 vsi_mk_dte(u32 dte)
+{
+	return (phys_addr_t)dte | VSI_DTE_PT_VALID;
+}
+
+#define VSI_PTE_PAGE_WRITABLE		BIT(2)
+#define VSI_PTE_PAGE_VALID		BIT(0)
+
+static inline phys_addr_t vsi_pte_page_address(u64 pte)
+{
+	return ((pte << VSI_PAGE_DESC_HI_SHIFT) & VSI_PAGE_DESC_HI_MASK) |
+	       (pte & VSI_PAGE_DESC_LO_MASK);
+}
+
+static u32 vsi_mk_pte(phys_addr_t page, int prot)
+{
+	u32 flags = 0;
+
+	flags |= (prot & IOMMU_WRITE) ? VSI_PTE_PAGE_WRITABLE : 0;
+
+	page = (page & VSI_PAGE_DESC_LO_MASK) |
+	       ((page & VSI_PAGE_DESC_HI_MASK) >> VSI_PAGE_DESC_HI_SHIFT);
+
+	return page | flags | VSI_PTE_PAGE_VALID;
+}
+
+#define VSI_DTE_PT_VALID	BIT(0)
+
+static inline bool vsi_dte_is_pt_valid(u32 dte)
+{
+	return dte & VSI_DTE_PT_VALID;
+}
+
+static inline bool vsi_pte_is_page_valid(u32 pte)
+{
+	return pte & VSI_PTE_PAGE_VALID;
+}
+
+static u32 vsi_mk_pte_invalid(u32 pte)
+{
+	return pte & ~VSI_PTE_PAGE_VALID;
+}
+
+#define VSI_MASTER_TLB_MASK	GENMASK_ULL(31, 10)
+/* mode 0 : 4k */
+#define VSI_PTA_4K_MODE	0
+
+static u64 vsi_mk_pta(dma_addr_t dt_dma)
+{
+	u64 val = (dt_dma & VSI_MASTER_TLB_MASK) | VSI_PTA_4K_MODE;
+
+	return val;
+}
+
+static struct vsi_iommu_domain *to_vsi_domain(struct iommu_domain *dom)
+{
+	return container_of(dom, struct vsi_iommu_domain, domain);
+}
+
+static inline void vsi_table_flush(struct vsi_iommu_domain *vsi_domain, dma_addr_t dma,
+				   unsigned int count)
+{
+	size_t size = count * sizeof(u32); /* count of u32 entry */
+
+	dma_sync_single_for_device(vsi_domain->dev, dma, size, DMA_TO_DEVICE);
+}
+
+#define VSI_IOVA_DTE_MASK	0xffc00000
+#define VSI_IOVA_DTE_SHIFT	22
+#define VSI_IOVA_PTE_MASK	0x003ff000
+#define VSI_IOVA_PTE_SHIFT	12
+#define VSI_IOVA_PAGE_MASK	0x00000fff
+#define VSI_IOVA_PAGE_SHIFT	0
+
+static u32 vsi_iova_dte_index(u32 iova)
+{
+	return (iova & VSI_IOVA_DTE_MASK) >> VSI_IOVA_DTE_SHIFT;
+}
+
+static u32 vsi_iova_pte_index(u32 iova)
+{
+	return (iova & VSI_IOVA_PTE_MASK) >> VSI_IOVA_PTE_SHIFT;
+}
+
+static u32 vsi_iova_page_offset(u32 iova)
+{
+	return (iova & VSI_IOVA_PAGE_MASK) >> VSI_IOVA_PAGE_SHIFT;
+}
+
+static void vsi_iommu_flush_tlb_all(struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	struct list_head *pos;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	list_for_each(pos, &vsi_domain->iommus) {
+		struct vsi_iommu *iommu;
+		int ret;
+
+		iommu = list_entry(pos, struct vsi_iommu, node);
+		ret = pm_runtime_resume_and_get(iommu->dev);
+		if (ret < 0)
+			continue;
+
+		spin_lock(&iommu->lock);
+
+		writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
+		writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
+
+		spin_unlock(&iommu->lock);
+		pm_runtime_put_autosuspend(iommu->dev);
+	}
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+}
+
+static irqreturn_t vsi_iommu_irq(int irq, void *dev_id)
+{
+	struct vsi_iommu *iommu = dev_id;
+	unsigned long flags;
+	dma_addr_t iova;
+	u32 status;
+
+	if (pm_runtime_resume_and_get(iommu->dev) < 0)
+		return IRQ_NONE;
+
+	spin_lock_irqsave(&iommu->lock, flags);
+
+	status = readl(iommu->regs + VSI_MMU_STATUS_BASE);
+	if (status & VSI_MMU_IRQ_MASK) {
+		dev_err(iommu->dev, "unexpected int_status=%08x\n", status);
+		iova = readl(iommu->regs + VSI_MMU_PAGE_FAULT_ADDR);
+		report_iommu_fault(iommu->domain, iommu->dev, iova, status);
+	}
+	writel(0, iommu->regs + VSI_MMU_STATUS_BASE);
+
+	spin_unlock_irqrestore(&iommu->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+
+	return IRQ_HANDLED;
+}
+
+static struct vsi_iommu *vsi_iommu_get_from_dev(struct device *dev)
+{
+	struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+	struct device *iommu_dev = bus_find_device_by_fwnode(&platform_bus_type,
+							     fwspec->iommu_fwnode);
+
+	put_device(iommu_dev);
+
+	return iommu_dev ? dev_get_drvdata(iommu_dev) : NULL;
+}
+
+static struct iommu_domain *vsi_iommu_domain_alloc_paging(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	struct vsi_iommu_domain *vsi_domain;
+
+	vsi_domain = kzalloc(sizeof(*vsi_domain), GFP_KERNEL);
+	if (!vsi_domain)
+		return NULL;
+
+	vsi_domain->dev = iommu->dev;
+	spin_lock_init(&vsi_domain->lock);
+
+	/*
+	 * iommu use a 2 level pagetable.
+	 * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
+	 * Allocate one 4 KiB page for each table.
+	 */
+	vsi_domain->dt = iommu_alloc_pages_sz(GFP_KERNEL | GFP_DMA32,
+					      SPAGE_SIZE);
+	if (!vsi_domain->dt)
+		goto err_free_domain;
+
+	vsi_domain->dt_dma = dma_map_single(vsi_domain->dev, vsi_domain->dt,
+					    SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, vsi_domain->dt_dma)) {
+		dev_err(dev, "DMA map error for DT\n");
+		goto err_free_dt;
+	}
+
+	vsi_domain->pta = iommu_alloc_pages_sz(GFP_KERNEL | GFP_DMA32,
+					       SPAGE_SIZE);
+	if (!vsi_domain->pta)
+		goto err_unmap_dt;
+
+	vsi_domain->pta[0] = vsi_mk_pta(vsi_domain->dt_dma);
+	vsi_domain->pta_dma = dma_map_single(vsi_domain->dev, vsi_domain->pta,
+					     SPAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, vsi_domain->pta_dma)) {
+		dev_err(dev, "DMA map error for PTA\n");
+		goto err_free_pta;
+	}
+
+	INIT_LIST_HEAD(&vsi_domain->iommus);
+
+	vsi_domain->domain.geometry.aperture_start = 0;
+	vsi_domain->domain.geometry.aperture_end   = DMA_BIT_MASK(32);
+	vsi_domain->domain.geometry.force_aperture = true;
+	vsi_domain->domain.pgsize_bitmap	   = SZ_4K;
+
+	return &vsi_domain->domain;
+
+err_free_pta:
+	iommu_free_pages(vsi_domain->pta);
+err_unmap_dt:
+	dma_unmap_single(vsi_domain->dev, vsi_domain->dt_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+err_free_dt:
+	iommu_free_pages(vsi_domain->dt);
+err_free_domain:
+	kfree(vsi_domain);
+
+	return NULL;
+}
+
+static phys_addr_t vsi_iommu_iova_to_phys(struct iommu_domain *domain,
+					  dma_addr_t iova)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	phys_addr_t pt_phys, phys = 0;
+	unsigned long flags;
+	u32 dte, pte;
+	u32 *page_table;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	if (!vsi_dte_is_pt_valid(dte))
+		goto unlock;
+
+	pt_phys = vsi_dte_pt_address(dte);
+	page_table = (u32 *)phys_to_virt(pt_phys);
+	pte = page_table[vsi_iova_pte_index(iova)];
+	if (!vsi_pte_is_page_valid(pte))
+		goto unlock;
+
+	phys = vsi_pte_page_address(pte) + vsi_iova_page_offset(iova);
+
+unlock:
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	return phys;
+}
+
+static size_t vsi_iommu_unmap_iova(struct vsi_iommu_domain *vsi_domain,
+				   u32 *pte_addr, dma_addr_t pte_dma,
+				   size_t size)
+{
+	unsigned int pte_count;
+	unsigned int pte_total = size / SPAGE_SIZE;
+
+	for (pte_count = 0;
+	     pte_count < pte_total && pte_count < NUM_PT_ENTRIES; pte_count++) {
+		u32 pte = pte_addr[pte_count];
+
+		if (!vsi_pte_is_page_valid(pte))
+			break;
+
+		pte_addr[pte_count] = vsi_mk_pte_invalid(pte);
+	}
+
+	vsi_table_flush(vsi_domain, pte_dma, pte_total);
+
+	return pte_count * SPAGE_SIZE;
+}
+
+static int vsi_iommu_map_iova(struct vsi_iommu_domain *vsi_domain, u32 *pte_addr,
+			      dma_addr_t pte_dma, dma_addr_t iova,
+			      phys_addr_t paddr, size_t size, int prot)
+{
+	unsigned int pte_count;
+	unsigned int pte_total = size / SPAGE_SIZE;
+
+	for (pte_count = 0;
+	     pte_count < pte_total && pte_count < NUM_PT_ENTRIES; pte_count++) {
+		u32 pte = pte_addr[pte_count];
+
+		if (vsi_pte_is_page_valid(pte))
+			return (pte_count - 1) * SPAGE_SIZE;
+
+		pte_addr[pte_count] = vsi_mk_pte(paddr, prot);
+
+		paddr += SPAGE_SIZE;
+	}
+
+	vsi_table_flush(vsi_domain, pte_dma, pte_total);
+
+	return 0;
+}
+
+static size_t vsi_iommu_unmap(struct iommu_domain *domain, unsigned long _iova,
+			      size_t size, size_t count, struct iommu_iotlb_gather *gather)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
+	unsigned long flags;
+	phys_addr_t pt_phys;
+	u32 dte;
+	u32 *pte_addr;
+	size_t unmap_size = 0;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	/* Just return 0 if iova is unmapped */
+	if (!vsi_dte_is_pt_valid(dte))
+		goto unlock;
+
+	pt_phys = vsi_dte_pt_address(dte);
+	pte_addr = (u32 *)phys_to_virt(pt_phys) + vsi_iova_pte_index(iova);
+	pte_dma = pt_phys + vsi_iova_pte_index(iova) * sizeof(u32);
+	unmap_size = vsi_iommu_unmap_iova(vsi_domain, pte_addr, pte_dma, size);
+
+unlock:
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	return unmap_size;
+}
+
+static u32 *vsi_dte_get_page_table(struct vsi_iommu_domain *vsi_domain,
+				   dma_addr_t iova, gfp_t gfp)
+{
+	u32 *page_table, *dte_addr;
+	u32 dte_index, dte;
+	phys_addr_t pt_phys;
+	dma_addr_t pt_dma;
+	gfp_t flags;
+
+	dte_index = vsi_iova_dte_index(iova);
+	dte_addr = &vsi_domain->dt[dte_index];
+	dte = *dte_addr;
+	if (vsi_dte_is_pt_valid(dte))
+		goto done;
+
+	/* Do not allow to sleep while allocating the buffer */
+	flags = (gfp & ~GFP_KERNEL) | GFP_ATOMIC | GFP_DMA32;
+	page_table = iommu_alloc_pages_sz(flags, PAGE_SIZE);
+	if (!page_table)
+		return ERR_PTR(-ENOMEM);
+
+	pt_dma = dma_map_single(vsi_domain->dev, page_table, PAGE_SIZE, DMA_TO_DEVICE);
+	if (dma_mapping_error(vsi_domain->dev, pt_dma)) {
+		dev_err(vsi_domain->dev, "DMA mapping error while allocating page table\n");
+		iommu_free_pages(page_table);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	dte = vsi_mk_dte(pt_dma);
+	*dte_addr = dte;
+
+	vsi_table_flush(vsi_domain,
+			vsi_domain->dt_dma + dte_index * sizeof(u32), 1);
+done:
+	pt_phys = vsi_dte_pt_address(dte);
+	return (u32 *)phys_to_virt(pt_phys);
+}
+
+static int vsi_iommu_map(struct iommu_domain *domain, unsigned long _iova,
+			 phys_addr_t paddr, size_t size, size_t count,
+			 int prot, gfp_t gfp, size_t *mapped)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	dma_addr_t pte_dma, iova = (dma_addr_t)_iova;
+	u32 *page_table, *pte_addr;
+	u32 dte, pte_index;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	page_table = vsi_dte_get_page_table(vsi_domain, iova, gfp);
+	if (IS_ERR(page_table)) {
+		spin_unlock_irqrestore(&vsi_domain->lock, flags);
+		return PTR_ERR(page_table);
+	}
+
+	dte = vsi_domain->dt[vsi_iova_dte_index(iova)];
+	pte_index = vsi_iova_pte_index(iova);
+	pte_addr = &page_table[pte_index];
+	pte_dma = vsi_dte_pt_address(dte) + pte_index * sizeof(u32);
+	ret = vsi_iommu_map_iova(vsi_domain, pte_addr, pte_dma, iova,
+				 paddr, size, prot);
+	if (!ret)
+		*mapped = size;
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	return ret;
+}
+
+static void vsi_iommu_disable(struct vsi_iommu *iommu)
+{
+	writel(0, iommu->regs + VSI_MMU_AHB_CONTROL_BASE);
+}
+
+static int vsi_iommu_identity_attach(struct iommu_domain *domain,
+				     struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	unsigned long flags;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(iommu->dev);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irqsave(&iommu->lock, flags);
+	if (iommu->domain == domain)
+		goto unlock;
+
+	vsi_iommu_disable(iommu);
+	list_del_init(&iommu->node);
+
+	iommu->domain = domain;
+
+unlock:
+	spin_unlock_irqrestore(&iommu->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+	return 0;
+}
+
+static const struct iommu_domain_ops vsi_identity_ops = {
+	.attach_dev = vsi_iommu_identity_attach,
+};
+
+static struct iommu_domain vsi_identity_domain = {
+	.type = IOMMU_DOMAIN_IDENTITY,
+	.ops = &vsi_identity_ops,
+};
+
+static void vsi_iommu_enable(struct vsi_iommu *iommu, struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+
+	if (domain == &vsi_identity_domain)
+		return;
+
+	writel(vsi_domain->pta_dma, iommu->regs + VSI_MMU_AHB_TLB_ARRAY_BASE_L_BASE);
+	writel(VSI_MMU_OUT_OF_BOUND, iommu->regs + VSI_MMU_CONFIG1_BASE);
+	writel(VSI_MMU_BIT_ENABLE, iommu->regs + VSI_MMU_AHB_EXCEPTION_BASE);
+	writel(VSI_MMU_BIT_ENABLE, iommu->regs + VSI_MMU_AHB_CONTROL_BASE);
+}
+
+void vsi_iommu_restore_ctx(struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	struct list_head *pos;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	list_for_each(pos, &vsi_domain->iommus) {
+		struct vsi_iommu *iommu;
+
+		iommu = list_entry(pos, struct vsi_iommu, node);
+		if (!iommu)
+			continue;
+
+		spin_lock(&iommu->lock);
+
+		writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
+		writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
+
+		spin_unlock(&iommu->lock);
+	}
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+}
+EXPORT_SYMBOL_GPL(vsi_iommu_restore_ctx);
+
+static int vsi_iommu_attach_device(struct iommu_domain *domain,
+				   struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	unsigned long flags, flags2;
+	int ret = 0;
+
+	ret = pm_runtime_resume_and_get(iommu->dev);
+	if (ret < 0)
+		return ret;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+	spin_lock_irqsave(&iommu->lock, flags2);
+
+	vsi_iommu_enable(iommu, domain);
+	writel(VSI_MMU_BIT_FLUSH, iommu->regs + VSI_MMU_FLUSH_BASE);
+	writel(0, iommu->regs + VSI_MMU_FLUSH_BASE);
+
+	list_del_init(&iommu->node);
+	list_add_tail(&iommu->node, &vsi_domain->iommus);
+
+	iommu->domain = domain;
+
+	spin_unlock_irqrestore(&iommu->lock, flags2);
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	pm_runtime_put_autosuspend(iommu->dev);
+	return ret;
+}
+
+static void vsi_iommu_domain_free(struct iommu_domain *domain)
+{
+	struct vsi_iommu_domain *vsi_domain = to_vsi_domain(domain);
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&vsi_domain->lock, flags);
+
+	WARN_ON(!list_empty(&vsi_domain->iommus));
+
+	for (i = 0; i < NUM_DT_ENTRIES; i++) {
+		u32 dte = vsi_domain->dt[i];
+
+		if (vsi_dte_is_pt_valid(dte)) {
+			phys_addr_t pt_phys = vsi_dte_pt_address(dte);
+			u32 *page_table = phys_to_virt(pt_phys);
+
+			dma_unmap_single(vsi_domain->dev, pt_phys,
+					 SPAGE_SIZE, DMA_TO_DEVICE);
+			iommu_free_pages(page_table);
+		}
+	}
+
+	dma_unmap_single(vsi_domain->dev, vsi_domain->dt_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+	iommu_free_pages(vsi_domain->dt);
+
+	dma_unmap_single(vsi_domain->dev, vsi_domain->pta_dma,
+			 SPAGE_SIZE, DMA_TO_DEVICE);
+	iommu_free_pages(vsi_domain->pta);
+
+	spin_unlock_irqrestore(&vsi_domain->lock, flags);
+
+	kfree(vsi_domain);
+}
+
+static struct iommu_device *vsi_iommu_probe_device(struct device *dev)
+{
+	struct vsi_iommu *iommu = vsi_iommu_get_from_dev(dev);
+	struct device_link *link;
+
+	link = device_link_add(dev, iommu->dev,
+			       DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME);
+	if (!link)
+		dev_err(dev, "Unable to link %s\n", dev_name(iommu->dev));
+
+	dev_iommu_priv_set(dev, iommu);
+	return &iommu->iommu;
+}
+
+static void vsi_iommu_release_device(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_iommu_priv_get(dev);
+
+	device_link_remove(dev, iommu->dev);
+}
+
+static int vsi_iommu_of_xlate(struct device *dev, const struct of_phandle_args *args)
+{
+	return iommu_fwspec_add_ids(dev, args->args, 1);
+}
+
+static const struct iommu_ops vsi_iommu_ops = {
+	.identity_domain = &vsi_identity_domain,
+	.release_domain = &vsi_identity_domain,
+	.domain_alloc_paging = vsi_iommu_domain_alloc_paging,
+	.of_xlate = vsi_iommu_of_xlate,
+	.probe_device = vsi_iommu_probe_device,
+	.release_device = vsi_iommu_release_device,
+	.device_group = generic_single_device_group,
+	.owner = THIS_MODULE,
+	.default_domain_ops = &(const struct iommu_domain_ops) {
+		.attach_dev		= vsi_iommu_attach_device,
+		.map_pages		= vsi_iommu_map,
+		.unmap_pages		= vsi_iommu_unmap,
+		.flush_iotlb_all	= vsi_iommu_flush_tlb_all,
+		.iova_to_phys		= vsi_iommu_iova_to_phys,
+		.free			= vsi_iommu_domain_free,
+	}
+};
+
+static const struct of_device_id vsi_iommu_dt_ids[] = {
+	{
+		.compatible = "verisilicon,iommu-1.2",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, vsi_iommu_dt_ids);
+
+static int vsi_iommu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct vsi_iommu *iommu;
+	int err;
+
+	iommu = devm_kzalloc(dev, sizeof(*iommu), GFP_KERNEL);
+	if (!iommu)
+		return -ENOMEM;
+
+	iommu->dev = dev;
+	spin_lock_init(&iommu->lock);
+	INIT_LIST_HEAD(&iommu->node);
+
+	iommu->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(iommu->regs))
+		return -ENOMEM;
+
+	iommu->num_clocks = devm_clk_bulk_get_all(dev, &iommu->clocks);
+	if  (iommu->num_clocks < 0)
+		return iommu->num_clocks;
+
+	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
+	if (err)
+		return err;
+
+	iommu->irq = platform_get_irq(pdev, 0);
+	if (iommu->irq < 0)
+		return iommu->irq;
+
+	err = devm_request_irq(iommu->dev, iommu->irq, vsi_iommu_irq,
+			       IRQF_SHARED, dev_name(dev), iommu);
+	if (err)
+		goto err_unprepare_clocks;
+
+	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+	platform_set_drvdata(pdev, iommu);
+
+	pm_runtime_set_autosuspend_delay(dev, 100);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_enable(dev);
+
+	err = iommu_device_sysfs_add(&iommu->iommu, dev, NULL, dev_name(dev));
+	if (err)
+		goto err_runtime_disable;
+
+	err = iommu_device_register(&iommu->iommu, &vsi_iommu_ops, dev);
+	if (err)
+		goto err_remove_sysfs;
+
+	return 0;
+
+err_remove_sysfs:
+	iommu_device_sysfs_remove(&iommu->iommu);
+err_runtime_disable:
+	pm_runtime_disable(dev);
+err_unprepare_clocks:
+	clk_bulk_unprepare(iommu->num_clocks, iommu->clocks);
+	return err;
+}
+
+static void vsi_iommu_shutdown(struct platform_device *pdev)
+{
+	struct vsi_iommu *iommu = platform_get_drvdata(pdev);
+
+	disable_irq(iommu->irq);
+	pm_runtime_force_suspend(&pdev->dev);
+}
+
+static int __maybe_unused vsi_iommu_suspend(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_get_drvdata(dev);
+
+	vsi_iommu_disable(iommu);
+
+	clk_bulk_disable(iommu->num_clocks, iommu->clocks);
+
+	return 0;
+}
+
+static int __maybe_unused vsi_iommu_resume(struct device *dev)
+{
+	struct vsi_iommu *iommu = dev_get_drvdata(dev);
+	unsigned long flags, flags2;
+	int ret;
+
+	ret = clk_bulk_enable(iommu->num_clocks, iommu->clocks);
+	if (ret)
+		return ret;
+
+	if (iommu->domain) {
+		struct vsi_iommu_domain *vsi_domain = to_vsi_domain(iommu->domain);
+
+		spin_lock_irqsave(&vsi_domain->lock, flags);
+		spin_lock_irqsave(&iommu->lock, flags2);
+		vsi_iommu_enable(iommu, iommu->domain);
+		spin_unlock_irqrestore(&iommu->lock, flags2);
+		spin_unlock_irqrestore(&vsi_domain->lock, flags);
+	}
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(vsi_iommu_pm_ops,
+				 vsi_iommu_suspend, vsi_iommu_resume,
+				 NULL);
+
+static struct platform_driver rockchip_vsi_iommu_driver = {
+	.probe = vsi_iommu_probe,
+	.shutdown = vsi_iommu_shutdown,
+	.driver = {
+		   .name = "vsi_iommu",
+		   .of_match_table = vsi_iommu_dt_ids,
+		   .pm = pm_sleep_ptr(&vsi_iommu_pm_ops),
+		   .suppress_bind_attrs = true,
+	},
+};
+module_platform_driver(rockchip_vsi_iommu_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Benjamin Gaignard <benjamin.gaignard@collabora.com>");
+MODULE_DESCRIPTION("Verisilicon IOMMU driver");
diff --git a/include/linux/vsi-iommu.h b/include/linux/vsi-iommu.h
new file mode 100644
index 000000000000..d7079bd8550e
--- /dev/null
+++ b/include/linux/vsi-iommu.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * verisilicon iommu: simple virtual address space management
+ *
+ * Copyright (c) 2025, Collabora
+ *
+ * Written by Benjamin Gaignard <benjamin.gaignard@collabora.com>
+ */
+
+#ifndef _VSI_IOMMU_H_
+#define _VSI_IOMMU_H_
+
+struct iommu_domain;
+
+#ifdef CONFIG_VSI_IOMMU
+void vsi_iommu_restore_ctx(struct iommu_domain *domain);
+#else
+static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
+#endif
+
+#endif
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 4/7] MAINTAINERS: Add entry for Verisilicon IOMMU driver
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
                   ` (2 preceding siblings ...)
  2025-09-11 15:57 ` [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 5/7] media: verisilicon: AV1: Restore IOMMU context before decoding a frame Benjamin Gaignard
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

Add maintainer for Verisilicon iommu driver.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6dcfbd11efef..023d0e8b370b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26407,6 +26407,14 @@ F:	drivers/media/i2c/vd55g1.c
 F:	drivers/media/i2c/vd56g3.c
 F:	drivers/media/i2c/vgxy61.c
 
+VERISILICON IOMMU DRIVER
+M:	Benjamin Gaignard <benjamin.gaignard@collabora.com>
+L:	iommu@lists.linux.dev
+S:	Maintained
+F:	Documentation/devicetree/bindings/iommu/verisilicon,iommu.yaml
+F:	drivers/iommu/vsi-iommu.c
+F:	include/linux/vsi-iommu.h
+
 VF610 NAND DRIVER
 M:	Stefan Agner <stefan@agner.ch>
 L:	linux-mtd@lists.infradead.org
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 5/7] media: verisilicon: AV1: Restore IOMMU context before decoding a frame
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
                   ` (3 preceding siblings ...)
  2025-09-11 15:57 ` [PATCH v9 4/7] MAINTAINERS: Add entry for Verisilicon " Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 6/7] arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588 Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 7/7] arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588 Benjamin Gaignard
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

AV1 is a stateless decoder which means multiple AV1 bitstreams could be
decoded at the same time using the same hardware block. Before decoding
a frame it is needed to restore the iommu tables to avoid mixing decode
contexts.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 .../verisilicon/rockchip_vpu981_hw_av1_dec.c      | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
index e4703bb6be7c..d9e68e0ded68 100644
--- a/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
+++ b/drivers/media/platform/verisilicon/rockchip_vpu981_hw_av1_dec.c
@@ -5,6 +5,9 @@
  * Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>
  */
 
+#include <linux/iommu.h>
+#include <linux/vsi-iommu.h>
+
 #include <media/v4l2-mem2mem.h>
 #include "hantro.h"
 #include "hantro_v4l2.h"
@@ -2095,12 +2098,24 @@ rockchip_vpu981_av1_dec_set_output_buffer(struct hantro_ctx *ctx)
 	hantro_write_addr(vpu, AV1_TILE_OUT_MV, mv_addr);
 }
 
+static void rockchip_vpu981_av1_restore_iommu(struct hantro_ctx *ctx)
+{
+	struct iommu_domain *domain;
+
+	/* Before decoding any frame iommu context need to be restored */
+	domain = iommu_get_domain_for_dev(ctx->dev->v4l2_dev.dev);
+	if (domain)
+		vsi_iommu_restore_ctx(domain);
+}
+
 int rockchip_vpu981_av1_dec_run(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *vb2_src;
 	int ret;
 
+	rockchip_vpu981_av1_restore_iommu(ctx);
+
 	hantro_start_prepare_run(ctx);
 
 	ret = rockchip_vpu981_av1_dec_prepare_run(ctx);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 6/7] arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
                   ` (4 preceding siblings ...)
  2025-09-11 15:57 ` [PATCH v9 5/7] media: verisilicon: AV1: Restore IOMMU context before decoding a frame Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  2025-09-11 15:57 ` [PATCH v9 7/7] arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588 Benjamin Gaignard
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

Add the device tree node for the Verisilicon IOMMU present
in the RK3588 SoC.
This IOMMU handles address translation for the VPU hardware blocks.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3588-base.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
index 70f03e68ba55..8656e46ad288 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
@@ -1263,6 +1263,17 @@ av1d: video-codec@fdc70000 {
 		clock-names = "aclk", "hclk";
 		power-domains = <&power RK3588_PD_AV1>;
 		resets = <&cru SRST_A_AV1>, <&cru SRST_P_AV1>, <&cru SRST_A_AV1_BIU>, <&cru SRST_P_AV1_BIU>;
+		iommus = <&av1d_mmu>;
+	};
+
+	av1d_mmu: iommu@fdca0000 {
+		compatible = "rockchip,rk3588-av1-iommu", "verisilicon,iommu-1.2";
+		reg = <0x0 0xfdca0000 0x0 0x600>;
+		interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH 0>;
+		clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+		clock-names = "core", "iface";
+		#iommu-cells = <0>;
+		power-domains = <&power RK3588_PD_AV1>;
 	};
 
 	vop: vop@fdd90000 {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v9 7/7] arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588
  2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
                   ` (5 preceding siblings ...)
  2025-09-11 15:57 ` [PATCH v9 6/7] arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588 Benjamin Gaignard
@ 2025-09-11 15:57 ` Benjamin Gaignard
  6 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-11 15:57 UTC (permalink / raw)
  To: joro, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	nicolas.dufresne, jgg, p.zabel, mchehab
  Cc: iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media, Benjamin Gaignard

Enable Verisilicon IOMMU used by Rockchip RK3588 AV1 hardware codec.
This hardware block could be found in Radxa Rock 5B board.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 58f87d09366c..04547bcc904b 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1470,6 +1470,7 @@ CONFIG_ARM_SMMU=y
 CONFIG_ARM_SMMU_V3=y
 CONFIG_MTK_IOMMU=y
 CONFIG_QCOM_IOMMU=y
+CONFIG_VSI_IOMMU=m
 CONFIG_REMOTEPROC=y
 CONFIG_IMX_REMOTEPROC=y
 CONFIG_MTK_SCP=m
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-11 15:57 ` [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver Benjamin Gaignard
@ 2025-09-12 11:47   ` Jörg Rödel
  2025-09-12 12:33     ` Benjamin Gaignard
  2025-09-12 17:37     ` Nicolas Dufresne
  2025-12-12  3:31   ` Jianfeng Liu
  1 sibling, 2 replies; 19+ messages in thread
From: Jörg Rödel @ 2025-09-12 11:47 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: robin.murphy, robh, krzk+dt, conor+dt, heiko, nicolas.dufresne,
	jgg, p.zabel, mchehab, iommu, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip, kernel, linux-media

On Thu, Sep 11, 2025 at 05:57:13PM +0200, Benjamin Gaignard wrote:
> The Verisilicon IOMMU hardware block can be found in combination
> with Verisilicon hardware video codecs (encoders or decoders) on
> different SoCs.
> Enable it will allow us to use non contiguous memory allocators
> for Verisilicon video codecs.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
>  drivers/iommu/Kconfig     |  11 +
>  drivers/iommu/Makefile    |   1 +
>  drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
>  include/linux/vsi-iommu.h |  21 +
>  4 files changed, 841 insertions(+)
>  create mode 100644 drivers/iommu/vsi-iommu.c
>  create mode 100644 include/linux/vsi-iommu.h

This will not go in before Will Deacons comment about code duplication with the
Rockchip driver is addressed.


	Joerg


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-12 11:47   ` Jörg Rödel
@ 2025-09-12 12:33     ` Benjamin Gaignard
  2025-09-12 17:37     ` Nicolas Dufresne
  1 sibling, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-12 12:33 UTC (permalink / raw)
  To: Jörg Rödel
  Cc: robin.murphy, robh, krzk+dt, conor+dt, heiko, nicolas.dufresne,
	jgg, p.zabel, mchehab, iommu, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip, kernel, linux-media


Le 12/09/2025 à 13:47, Jörg Rödel a écrit :
> On Thu, Sep 11, 2025 at 05:57:13PM +0200, Benjamin Gaignard wrote:
>> The Verisilicon IOMMU hardware block can be found in combination
>> with Verisilicon hardware video codecs (encoders or decoders) on
>> different SoCs.
>> Enable it will allow us to use non contiguous memory allocators
>> for Verisilicon video codecs.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
>> ---
>>   drivers/iommu/Kconfig     |  11 +
>>   drivers/iommu/Makefile    |   1 +
>>   drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
>>   include/linux/vsi-iommu.h |  21 +
>>   4 files changed, 841 insertions(+)
>>   create mode 100644 drivers/iommu/vsi-iommu.c
>>   create mode 100644 include/linux/vsi-iommu.h
> This will not go in before Will Deacons comment about code duplication with the
> Rockchip driver is addressed.

It isn't the same hardware block, not from the same IP vendor and the bits set aren't the same.
pm_runtime, clock, spin_lock aren't managed differently.
The common point is they both use 2-level page table.

VSI iommu need an additional function to restore iommu context before decoding
each frame, rockchip doesn't need it.

Regards,
Benjamin

>
> 	Joerg


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-12 11:47   ` Jörg Rödel
  2025-09-12 12:33     ` Benjamin Gaignard
@ 2025-09-12 17:37     ` Nicolas Dufresne
  2025-09-12 18:32       ` Jason Gunthorpe
  2025-09-13  5:58       ` Jörg Rödel
  1 sibling, 2 replies; 19+ messages in thread
From: Nicolas Dufresne @ 2025-09-12 17:37 UTC (permalink / raw)
  To: Jörg Rödel, Benjamin Gaignard
  Cc: robin.murphy, robh, krzk+dt, conor+dt, heiko, jgg, p.zabel,
	mchehab, iommu, devicetree, linux-kernel, linux-arm-kernel,
	linux-rockchip, kernel, linux-media

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

Hi,

Le vendredi 12 septembre 2025 à 13:47 +0200, Jörg Rödel a écrit :
> On Thu, Sep 11, 2025 at 05:57:13PM +0200, Benjamin Gaignard wrote:
> > The Verisilicon IOMMU hardware block can be found in combination
> > with Verisilicon hardware video codecs (encoders or decoders) on
> > different SoCs.
> > Enable it will allow us to use non contiguous memory allocators
> > for Verisilicon video codecs.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > ---
> >  drivers/iommu/Kconfig     |  11 +
> >  drivers/iommu/Makefile    |   1 +
> >  drivers/iommu/vsi-iommu.c | 808 ++++++++++++++++++++++++++++++++++++++
> >  include/linux/vsi-iommu.h |  21 +
> >  4 files changed, 841 insertions(+)
> >  create mode 100644 drivers/iommu/vsi-iommu.c
> >  create mode 100644 include/linux/vsi-iommu.h
> 
> This will not go in before Will Deacons comment about code duplication with the
> Rockchip driver is addressed.

So you'd like a new layer of ops to abstract the common parts of two drivers ? I
mean, I can see this happening for VSI/RK, but it seems rather pointless if in a
year from now Jason get his new framework ready. Re-doing the ops if they cause
too much duplicate seems like a better direction.

Benjamin already stated he'd be happy to port once there is enough example and
acceptability of the new code to do so. Why do mainline users have to suffer
this ?

To me this rejection isn't about Benjamin's driver, all iommu seems to look
alike, so anyone else that would have sent new driver would have face the same
issue.

Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-12 17:37     ` Nicolas Dufresne
@ 2025-09-12 18:32       ` Jason Gunthorpe
  2025-09-13  5:58       ` Jörg Rödel
  1 sibling, 0 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2025-09-12 18:32 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Jörg Rödel, Benjamin Gaignard, robin.murphy, robh,
	krzk+dt, conor+dt, heiko, p.zabel, mchehab, iommu, devicetree,
	linux-kernel, linux-arm-kernel, linux-rockchip, kernel,
	linux-media

On Fri, Sep 12, 2025 at 01:37:11PM -0400, Nicolas Dufresne wrote:

> To me this rejection isn't about Benjamin's driver, all iommu seems to look
> alike, so anyone else that would have sent new driver would have face the same
> issue.

Yes, many drivers looks approximately alike.. To the extent possible
we should be making more helpers and tools to lift common patterns out
of the drivers. I don't know how much of that applies to rockchip vs
verisilicon though. Obviously I'm tackling the page table stuff.

Much of the other common code is tracking attachments and invalidation
requirements. I'm hopeful the general method Nicolin is working on can
be lifted and shared. Certainly it is better than what is in any of
the three server focused drivers. But I don't know how much this
brings to these simpler drivers.

It seems strange to be talking about merging drivers that don't share
a common IP supplier or register programming model.

Jason


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-12 17:37     ` Nicolas Dufresne
  2025-09-12 18:32       ` Jason Gunthorpe
@ 2025-09-13  5:58       ` Jörg Rödel
  2025-09-15 22:58         ` Jason Gunthorpe
  1 sibling, 1 reply; 19+ messages in thread
From: Jörg Rödel @ 2025-09-13  5:58 UTC (permalink / raw)
  To: Nicolas Dufresne
  Cc: Benjamin Gaignard, robin.murphy, robh, krzk+dt, conor+dt, heiko,
	jgg, p.zabel, mchehab, iommu, devicetree, linux-kernel,
	linux-arm-kernel, linux-rockchip, kernel, linux-media,
	Will Deacon

[Adding Will back to Cc]

On Fri, Sep 12, 2025 at 01:37:11PM -0400, Nicolas Dufresne wrote:
> To me this rejection isn't about Benjamin's driver, all iommu seems to look
> alike, so anyone else that would have sent new driver would have face the same
> issue.

This is about ignoring comments from one of the IOMMU maintainers. I am not
going to merge a driver with open comments/objections[1] from Will (and a few
others), so resolve this with him and get his Ack.


	Joerg

[1] https://lore.kernel.org/all/aMAno-DkDJumcAtj@willie-the-truck/


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-13  5:58       ` Jörg Rödel
@ 2025-09-15 22:58         ` Jason Gunthorpe
  2025-09-16  8:48           ` Will Deacon
  0 siblings, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2025-09-15 22:58 UTC (permalink / raw)
  To: Jörg Rödel
  Cc: Nicolas Dufresne, Benjamin Gaignard, robin.murphy, robh, krzk+dt,
	conor+dt, heiko, p.zabel, mchehab, iommu, devicetree,
	linux-kernel, linux-arm-kernel, linux-rockchip, kernel,
	linux-media, Will Deacon

On Sat, Sep 13, 2025 at 07:58:04AM +0200, Jörg Rödel wrote:
> [Adding Will back to Cc]
> 
> On Fri, Sep 12, 2025 at 01:37:11PM -0400, Nicolas Dufresne wrote:
> > To me this rejection isn't about Benjamin's driver, all iommu seems to look
> > alike, so anyone else that would have sent new driver would have face the same
> > issue.
> 
> This is about ignoring comments from one of the IOMMU maintainers. I am not
> going to merge a driver with open comments/objections[1] from Will (and a few
> others), so resolve this with him and get his Ack.

I would strongly object to trying to share map_pages, unmap_pages,
iova_to_phys, free and other iommu pt related functions in some
limited way instead of helping on the much more complete iommu pt
work. Which is what I said to Will, but for some reason he suggested
it anyhow.

Sorry, but it doesn't make sense to complain about duplication in
drivers and then not help advance one of the biggest projects to
actually concretely and comprehensively address that duplication.

Jason


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-15 22:58         ` Jason Gunthorpe
@ 2025-09-16  8:48           ` Will Deacon
  2025-09-16  9:30             ` Benjamin Gaignard
  2025-09-16 13:02             ` Jason Gunthorpe
  0 siblings, 2 replies; 19+ messages in thread
From: Will Deacon @ 2025-09-16  8:48 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Jörg Rödel, Nicolas Dufresne, Benjamin Gaignard,
	robin.murphy, robh, krzk+dt, conor+dt, heiko, p.zabel, mchehab,
	iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media

On Mon, Sep 15, 2025 at 07:58:06PM -0300, Jason Gunthorpe wrote:
> On Sat, Sep 13, 2025 at 07:58:04AM +0200, Jörg Rödel wrote:
> > [Adding Will back to Cc]
> > 
> > On Fri, Sep 12, 2025 at 01:37:11PM -0400, Nicolas Dufresne wrote:
> > > To me this rejection isn't about Benjamin's driver, all iommu seems to look
> > > alike, so anyone else that would have sent new driver would have face the same
> > > issue.
> > 
> > This is about ignoring comments from one of the IOMMU maintainers. I am not
> > going to merge a driver with open comments/objections[1] from Will (and a few
> > others), so resolve this with him and get his Ack.
> 
> I would strongly object to trying to share map_pages, unmap_pages,
> iova_to_phys, free and other iommu pt related functions in some
> limited way instead of helping on the much more complete iommu pt
> work. Which is what I said to Will, but for some reason he suggested
> it anyhow.

If the answer is to convert this to iommu pt, then so be it. My
understanding was that was still premature at this stage but you know
better than me.

When I bothered to look at this driver side-by-side with the rockchip
driver which, despite apparently being totally different IP (honest!),
is *remarkably* similar, I summarised the similarity in the default
domain ops:

https://lore.kernel.org/all/aH5yR9CkYSJ4PaZV@willie-the-truck/

But rather than respond to that, Benjamin just sent a new version. I
was hoping for a bit more discussion...

> Sorry, but it doesn't make sense to complain about duplication in
> drivers and then not help advance one of the biggest projects to
> actually concretely and comprehensively address that duplication.

I don't think it needs to be one or the other. afaict, these drivers
should share the default domain ops and if the page-table code is using
iommu-pt then that's even better.

Will


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-16  8:48           ` Will Deacon
@ 2025-09-16  9:30             ` Benjamin Gaignard
  2025-09-16 13:02             ` Jason Gunthorpe
  1 sibling, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-09-16  9:30 UTC (permalink / raw)
  To: Will Deacon, Jason Gunthorpe
  Cc: Jörg Rödel, Nicolas Dufresne, robin.murphy, robh,
	krzk+dt, conor+dt, heiko, p.zabel, mchehab, iommu, devicetree,
	linux-kernel, linux-arm-kernel, linux-rockchip, kernel,
	linux-media


Le 16/09/2025 à 10:48, Will Deacon a écrit :
> On Mon, Sep 15, 2025 at 07:58:06PM -0300, Jason Gunthorpe wrote:
>> On Sat, Sep 13, 2025 at 07:58:04AM +0200, Jörg Rödel wrote:
>>> [Adding Will back to Cc]
>>>
>>> On Fri, Sep 12, 2025 at 01:37:11PM -0400, Nicolas Dufresne wrote:
>>>> To me this rejection isn't about Benjamin's driver, all iommu seems to look
>>>> alike, so anyone else that would have sent new driver would have face the same
>>>> issue.
>>> This is about ignoring comments from one of the IOMMU maintainers. I am not
>>> going to merge a driver with open comments/objections[1] from Will (and a few
>>> others), so resolve this with him and get his Ack.
>> I would strongly object to trying to share map_pages, unmap_pages,
>> iova_to_phys, free and other iommu pt related functions in some
>> limited way instead of helping on the much more complete iommu pt
>> work. Which is what I said to Will, but for some reason he suggested
>> it anyhow.
> If the answer is to convert this to iommu pt, then so be it. My
> understanding was that was still premature at this stage but you know
> better than me.
>
> When I bothered to look at this driver side-by-side with the rockchip
> driver which, despite apparently being totally different IP (honest!),
> is *remarkably* similar, I summarised the similarity in the default
> domain ops:
>
> https://lore.kernel.org/all/aH5yR9CkYSJ4PaZV@willie-the-truck/
>
> But rather than respond to that, Benjamin just sent a new version. I
> was hoping for a bit more discussion...

Sorry if that had offend, you it wasn't the purpose.

Where you see similarities in the pattern I see lot of differences
everywhere that will required to duplicate the functions for each
hardware (locking schema, bit definition, power management, enable/disable).

Since the v6 I have fix lot of locking and pm_runtime issue and add a
function, needed by the video decoder, to flush the TLB.

My first attempt when writing Verisilicon driver was to add a variant
(like for rk iommu v2 I had already done) to Rockchip driver but mixing or
sharing structures or functions between Rockchip and Verisilicon is just a
nightmare because it requires to add "if else" everywhere.

Benjamin

>
>> Sorry, but it doesn't make sense to complain about duplication in
>> drivers and then not help advance one of the biggest projects to
>> actually concretely and comprehensively address that duplication.
> I don't think it needs to be one or the other. afaict, these drivers
> should share the default domain ops and if the page-table code is using
> iommu-pt then that's even better.
>
> Will


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-16  8:48           ` Will Deacon
  2025-09-16  9:30             ` Benjamin Gaignard
@ 2025-09-16 13:02             ` Jason Gunthorpe
  1 sibling, 0 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2025-09-16 13:02 UTC (permalink / raw)
  To: Will Deacon
  Cc: Jörg Rödel, Nicolas Dufresne, Benjamin Gaignard,
	robin.murphy, robh, krzk+dt, conor+dt, heiko, p.zabel, mchehab,
	iommu, devicetree, linux-kernel, linux-arm-kernel, linux-rockchip,
	kernel, linux-media

On Tue, Sep 16, 2025 at 09:48:33AM +0100, Will Deacon wrote:
> > I would strongly object to trying to share map_pages, unmap_pages,
> > iova_to_phys, free and other iommu pt related functions in some
> > limited way instead of helping on the much more complete iommu pt
> > work. Which is what I said to Will, but for some reason he suggested
> > it anyhow.
> 
> If the answer is to convert this to iommu pt, then so be it. My
> understanding was that was still premature at this stage but you know
> better than me.

We are waiting for reviews, hopefull when people return from
vacation. You know how hard it is to wrangle people to review it. But
the code is all done, it has all been tested by multiple vendors now,
many people have projects blocked on it.

If this duplication is really important to you then you are welcome to
help review it and merge it. As is Benjamin

> When I bothered to look at this driver side-by-side with the rockchip
> driver which, despite apparently being totally different IP (honest!),
> is *remarkably* similar, I summarised the similarity in the default
> domain ops:

Presumably there are more HW control bits in the PTE than just valid
that we don't know about..

> I don't think it needs to be one or the other. afaict, these drivers
> should share the default domain ops and if the page-table code is using
> iommu-pt then that's even better.

Refactoring rockchip into an iopgtbl, and making it work with this
driver is quite alot of throw away work in my view. I would not ask
Benjamin to do this.

This is why I offered the compromise that if he writes and tests the
iommu pt version I will carry it and he can merge the driver
as-is. Hopefully in a few months it can all be sorted.

We can then attempt to convert rockchip as part of the followup
activity to try to move drivers to iommupt.

Jason


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-09-11 15:57 ` [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver Benjamin Gaignard
  2025-09-12 11:47   ` Jörg Rödel
@ 2025-12-12  3:31   ` Jianfeng Liu
  2025-12-12  8:51     ` Benjamin Gaignard
  1 sibling, 1 reply; 19+ messages in thread
From: Jianfeng Liu @ 2025-12-12  3:31 UTC (permalink / raw)
  To: benjamin.gaignard
  Cc: conor+dt, devicetree, heiko, iommu, jgg, joro, kernel, krzk+dt,
	linux-arm-kernel, linux-kernel, linux-media, linux-rockchip,
	mchehab, nicolas.dufresne, p.zabel, robh, robin.murphy

Hi,

On Thu, 11 Sep 2025 17:57:13 +0200, Benjamin Gaignard wrote:
>+#ifdef CONFIG_VSI_IOMMU
>+void vsi_iommu_restore_ctx(struct iommu_domain *domain);
>+#else
>+static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
>+#endif

After applying this patch to v6.18, I get error when building this driver
as module:

drivers/iommu/vsi-iommu.c:541:6: error: redefinition of 'vsi_iommu_restore_ctx'
  541 | void vsi_iommu_restore_ctx(struct iommu_domain *domain)
   |      ^~~~~~~~~~~~~~~~~~~~~
In file included from drivers/iommu/vsi-iommu.c:31:
./include/linux/vsi-iommu.h:18:20: note: previous definition of 'vsi_iommu_restore_ctx' with type 'void(struct iommu_domain *)'
18 | static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
   |                    ^~~~~~~~~~~~~~~~~~~~~

I have to use:
#if IS_ENABLED(CONFIG_VSI_IOMMU)
instead.

Best regards,
Jianfeng


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver
  2025-12-12  3:31   ` Jianfeng Liu
@ 2025-12-12  8:51     ` Benjamin Gaignard
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Gaignard @ 2025-12-12  8:51 UTC (permalink / raw)
  To: Jianfeng Liu
  Cc: conor+dt, devicetree, heiko, iommu, jgg, joro, kernel, krzk+dt,
	linux-arm-kernel, linux-kernel, linux-media, linux-rockchip,
	mchehab, nicolas.dufresne, p.zabel, robh, robin.murphy


Le 12/12/2025 à 04:31, Jianfeng Liu a écrit :
> Hi,
>
> On Thu, 11 Sep 2025 17:57:13 +0200, Benjamin Gaignard wrote:
>> +#ifdef CONFIG_VSI_IOMMU
>> +void vsi_iommu_restore_ctx(struct iommu_domain *domain);
>> +#else
>> +static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
>> +#endif
> After applying this patch to v6.18, I get error when building this driver
> as module:
>
> drivers/iommu/vsi-iommu.c:541:6: error: redefinition of 'vsi_iommu_restore_ctx'
>    541 | void vsi_iommu_restore_ctx(struct iommu_domain *domain)
>     |      ^~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/iommu/vsi-iommu.c:31:
> ./include/linux/vsi-iommu.h:18:20: note: previous definition of 'vsi_iommu_restore_ctx' with type 'void(struct iommu_domain *)'
> 18 | static inline void vsi_iommu_restore_ctx(struct iommu_domain *domain) {}
>     |                    ^~~~~~~~~~~~~~~~~~~~~
>
> I have to use:
> #if IS_ENABLED(CONFIG_VSI_IOMMU)
> instead.

I have fix in my internal v10 branch:
https://gitlab.collabora.com/linux/for-upstream/-/commits/verisilicon_mmu_v10
I don't plan to send this update because that will change nothing about code duplication remarks.

Thanks for the report.
Regards,
Benjamin

> Best regards,
> Jianfeng
>


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-12-12  8:52 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-11 15:57 [PATCH v9 0/7] Add support for Verisilicon IOMMU used by media codec blocks Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 1/7] dt-bindings: vendor-prefixes: Add Verisilicon Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 2/7] dt-bindings: iommu: verisilicon: Add binding for VSI IOMMU Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 3/7] iommu: Add verisilicon IOMMU driver Benjamin Gaignard
2025-09-12 11:47   ` Jörg Rödel
2025-09-12 12:33     ` Benjamin Gaignard
2025-09-12 17:37     ` Nicolas Dufresne
2025-09-12 18:32       ` Jason Gunthorpe
2025-09-13  5:58       ` Jörg Rödel
2025-09-15 22:58         ` Jason Gunthorpe
2025-09-16  8:48           ` Will Deacon
2025-09-16  9:30             ` Benjamin Gaignard
2025-09-16 13:02             ` Jason Gunthorpe
2025-12-12  3:31   ` Jianfeng Liu
2025-12-12  8:51     ` Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 4/7] MAINTAINERS: Add entry for Verisilicon " Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 5/7] media: verisilicon: AV1: Restore IOMMU context before decoding a frame Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 6/7] arm64: dts: rockchip: Add verisilicon IOMMU node on RK3588 Benjamin Gaignard
2025-09-11 15:57 ` [PATCH v9 7/7] arm64: defconfig: enable Verisilicon IOMMU for Rockchip RK3588 Benjamin Gaignard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox