[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALAqxLXBYvwZ9kiKSGBeO5f-eKi2DD14QtoZgFGyGd-B7EOPQA@mail.gmail.com>
Date: Tue, 18 Aug 2020 15:02:54 -0700
From: John Stultz <john.stultz@...aro.org>
To: Robin Murphy <robin.murphy@....com>
Cc: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
driverdevel <devel@...verdev.osuosl.org>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, Joerg Roedel <jroedel@...e.de>,
Manivannan Sadhasivam <mani@...nel.org>,
Chenfeng <puck.chen@...ilicon.com>, linuxarm@...wei.com,
Wei Xu <xuwei5@...ilicon.com>,
lkml <linux-kernel@...r.kernel.org>,
iommu@...ts.linux-foundation.org, Rob Herring <robh+dt@...nel.org>,
mauro.chehab@...wei.com, Suzhuangluan <suzhuangluan@...ilicon.com>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@....com> 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@...00000 {
> > 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
Powered by blists - more mailing lists