On 2020-08-18 16:29, Mauro Carvalho Chehab wrote:
Hi Robin,
Em Tue, 18 Aug 2020 15:47:55 +0100
Robin Murphy <robin.murphy@xxxxxxx> 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.
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).
Robin.
