resend for switching to plain text mode. On Wed, 26 May 2021 at 15:59, Chunyan Zhang <zhang.lyra@xxxxxxxxx> wrote: > > 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.
