lkml.org 
[lkml]   [2020]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 00/16] IOMMU driver for Kirin 960/970
    Hi Robin,

    Em Tue, 18 Aug 2020 15:47:55 +0100
    Robin Murphy <robin.murphy@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

    \
     
     \ /
      Last update: 2020-08-18 17:30    [W:4.218 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site