Re: [PATCH 00/16] IOMMU driver for Kirin 960/970

[John Stultz]

On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
> On 2020-08-18 16:29, Mauro Carvalho Chehab wrote:
> > Em Tue, 18 Aug 2020 15:47:55 +0100
> > Basically, the DT binding has this, for IOMMU:
> >
> >
> >       smmu_lpae {
> >               compatible = "hisilicon,smmu-lpae";
> >       };
> >
> > ...
> >       dpe: dpe@e8600000 {
> >               compatible = "hisilicon,kirin970-dpe";
> >               memory-region = <&drm_dma_reserved>;
> > ...
> >               iommu_info {
> >                       start-addr = <0x8000>;
> >                       size = <0xbfff8000>;
> >               };
> >       }
> >
> > This is used by kirin9xx_drm_dss.c in order to enable and use
> > the iommu:
> >
> >
> >       static int dss_enable_iommu(struct platform_device *pdev, struct dss_hw_ctx *ctx)
> >       {
> >               struct device *dev = NULL;
> >
> >               dev = &pdev->dev;
> >
> >               /* create iommu domain */
> >               ctx->mmu_domain = iommu_domain_alloc(dev->bus);
> >               if (!ctx->mmu_domain) {
> >                       pr_err("iommu_domain_alloc failed!\n");
> >                       return -EINVAL;
> >               }
> >
> >               iommu_attach_device(ctx->mmu_domain, dev);
> >
> >               return 0;
> >       }
> >
> > The only place where the IOMMU domain is used is on this part of the
> > code(error part simplified here) [1]:
> >
> >       void hisi_dss_smmu_on(struct dss_hw_ctx *ctx)
> >       {
> >               uint64_t fama_phy_pgd_base;
> >               uint32_t phy_pgd_base;
> > ...
> >               fama_phy_pgd_base = iommu_iova_to_phys(ctx->mmu_domain, 0);
> >               phy_pgd_base = (uint32_t)fama_phy_pgd_base;
> >               if (WARN_ON(!phy_pgd_base))
> >                       return;
> >
> >               set_reg(smmu_base + SMMU_CB_TTBR0, phy_pgd_base, 32, 0);
> >       }
> >
> > [1] https://github.com/mchehab/linux/commit/36da105e719b47bbe9d6cb7e5619b30c7f3eb1bd
> >
> > In other words, the driver needs to get the physical address of the frame
> > buffer (mapped via iommu) in order to set some DRM-specific register.
> >
> > Yeah, the above code is somewhat hackish. I would love to replace
> > this part by a more standard approach.
>
> OK, so from a quick look at that, my impression is that your display
> controller has its own MMU and you don't need to pretend to use the
> IOMMU API at all. Just have the DRM driver use io-pgtable directly to
> run its own set of ARM_32_LPAE_S1 pagetables - see Panfrost for an
> example (but try to ignore the wacky "Mali LPAE" format).

Yea. For the HiKey960, there was originally a similar patch series but
it was refactored out and the (still out of tree) DRM driver I'm
carrying doesn't seem to need it (though looking we still have the
iommu_info subnode in the dts that maybe needs to be cleaned up).

thanks
-john