Hi Robin,
On Tue, 18 May 2021 at 00:35, Robin Murphy <robin.murphy@xxxxxxx> wrote:
On 2021-05-17 10:27, Joerg Roedel wrote:
> On Fri, Apr 30, 2021 at 08:20:10PM +0800, Kevin Tang wrote:
>> Cc Robin & Joerg
>
> This is just some GPU internal MMU being used here, it seems. It doesn't
> use the IOMMU core code, so no Ack needed from the IOMMU side.
Except the actual MMU being used is drivers/iommu/sprd_iommu.c - this is
Yes, it is using drivers/iommu/sprd_iommu.c.
just the display driver poking directly at the interrupt registers of
its associated IOMMU instance.
Actually the display driver is poking its own interrupt registers in which some interrupts are caused by using IOMMU, others are purely its own ones:
+/* Interrupt control & status bits */+#define BIT_DPU_INT_DONE BIT(0)
+#define BIT_DPU_INT_TE BIT(1)
+#define BIT_DPU_INT_ERR BIT(2)
+#define BIT_DPU_INT_UPDATE_DONE BIT(4)
+#define BIT_DPU_INT_VSYNC BIT(5)
+#define BIT_DPU_INT_MMU_VAOR_RD BIT(16)
+#define BIT_DPU_INT_MMU_VAOR_WR BIT(17)
+#define BIT_DPU_INT_MMU_INV_RD BIT(18)
+#define BIT_DPU_INT_MMU_INV_WR BIT(19)
From what I see in the product code, along with the information my colleagues told me, these _INT_MMU_ interrupts only need to be dealt with by client devices(i.e. display). IOMMU doesn't even have the INT_STS register for some early products which we're trying to support in the mainstream kernel.
I still think this is wrong, and that it
should be treated as a shared interrupt, with the IOMMU driver handling
its own registers and reporting to the client through the standard
report_iommu_fault() API, especially since there are apparently more
blocks using these IOMMU instances than just the display.
For the next generation IOMMU, we will handle interrupts in IOMMU drivers like you say here.
But like I explained above, we have to leave interrupt handling in the client device driver since the IOMMU we 're using in this display device doesn't have an INT_STS register in the IOMMU register range.
Thanks for the review,
Chunyan
Robin.
