Hi Robin,

Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy <robin.mur...@arm.com> escreveu:

> On 2020-08-17 08:49, Mauro Carvalho Chehab wrote:
> > Add a driver for the Kirin 960/970 iommu.
> > 
> > As on the past series, this starts from the original 4.9 driver from
> > the 96boards tree:
> > 
> >     https://github.com/96boards-hikey/linux/tree/hikey970-v4.9
> > 
> > The remaining patches add SPDX headers and make it build and run with
> > the upstream Kernel.
> > 
> > Chenfeng (1):
> >    iommu: add support for HiSilicon Kirin 960/970 iommu
> > 
> > Mauro Carvalho Chehab (15):
> >    iommu: hisilicon: remove default iommu_map_sg handler
> >    iommu: hisilicon: map and unmap ops gained new arguments
> >    iommu: hisi_smmu_lpae: rebase it to work with upstream
> >    iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h
> >    iommu: hisilicon: cleanup its code style
> >    iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE
> >    iommu: get rid of map/unmap tile functions
> >    iommu: hisi_smmu_lpae: use the right code to get domain-priv data
> >    iommu: hisi_smmu_lpae: convert it to probe_device
> >    iommu: add Hisilicon Kirin970 iommu at the building system
> >    iommu: hisi_smmu_lpae: cleanup printk macros
> >    iommu: hisi_smmu_lpae: make OF compatible more standard  
> 
> Echoing the other comments about none of the driver patches being CC'd 
> to the IOMMU list...
> 
> Still, I dug the series up on lore and frankly I'm not sure what to make 
> of it - AFAICS the "driver" is just yet another implementation of Arm 
> LPAE pagetable code, with no obvious indication of how those pagetables 
> ever get handed off to IOMMU hardware (and indeed no indication of IOMMU 
> hardware at all). Can you explain how it's supposed to work?
> 
> And as a pre-emptive strike, we really don't need any more LPAE 
> implementations - that's what the io-pgtable library is all about (which 
> incidentally has been around since 4.0...). I think that should make the 
> issue of preserving authorship largely moot since there's no need to 
> preserve most of the code anyway ;)

I didn't know about that, since I got a Hikey 970 board for the first time
about one month ago, and that's the first time I looked into iommu code.

My end goal with this is to make the DRM/KMS driver to work with upstream
Kernels.

The full patch series are at:

        https://github.com/mchehab/linux/commits/hikey970/to_upstream-2.0-v1.1

(I need to put a new version there, after some changes due to recent
upstream discussions at the regulator's part of the code)

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.

Thanks,
Mauro
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to