Re: [PATCH v5 4/6] drm/sprd: add Unisoc's drm display controller driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux