* [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 7:49 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:49 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, Rob Herring, linux-arm-kernel, Wei Xu,
devicetree, Joerg Roedel, Joerg Roedel, iommu, Chenfeng, devel,
Suzhuangluan, linux-kernel
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
dt: add an spec for the Kirin36x0 SMMU
dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
staging: hikey9xx: add an item about the iommu driver
.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++
.../boot/dts/hisilicon/hi3670-hikey970.dts | 3 +
drivers/staging/hikey9xx/Kconfig | 9 +
drivers/staging/hikey9xx/Makefile | 1 +
drivers/staging/hikey9xx/TODO | 1 +
drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++
7 files changed, 913 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
--
2.26.2
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 7:49 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:49 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Mauro Carvalho Chehab, Chenfeng, Joerg Roedel, linuxarm, Wei Xu,
linux-kernel, iommu, Rob Herring, John Stultz, mauro.chehab,
Suzhuangluan, linux-arm-kernel
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
dt: add an spec for the Kirin36x0 SMMU
dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
staging: hikey9xx: add an item about the iommu driver
.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++
.../boot/dts/hisilicon/hi3670-hikey970.dts | 3 +
drivers/staging/hikey9xx/Kconfig | 9 +
drivers/staging/hikey9xx/Makefile | 1 +
drivers/staging/hikey9xx/TODO | 1 +
drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++
7 files changed, 913 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* [PATCH 01/16] iommu: add support for HiSilicon Kirin 960/970 iommu
2020-08-17 7:49 ` Mauro Carvalho Chehab
(?)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Chenfeng, Mauro Carvalho Chehab,
John Stultz, Manivannan Sadhasivam, Suzhuangluan, linux-kernel,
devel
From: Chenfeng <puck.chen@hisilicon.com>
Add the IOMMU code used on Hikey 960/970 and required for its
DRM/KMS driver.
[john.stultz@linaro.org: split out all the ion changes, and kept just the iommu bits]
[mchehab+huawei@kernel.org: dropped ION and test code]
Signed-off-by: Chenfeng <puck.chen@hisilicon.com>
Reviewed-by: Suzhuangluan <suzhuangluan@hisilicon.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 178 +++++
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 849 ++++++++++++++++++++++
include/linux/hisi/hisi-iommu.h | 13 +
3 files changed, 1040 insertions(+)
create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
create mode 100644 include/linux/hisi/hisi-iommu.h
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
new file mode 100644
index 000000000000..4637244dba6b
--- /dev/null
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -0,0 +1,178 @@
+#ifndef HISI_SMMU_H
+#define HISI_SMMU_H
+
+/*#define IOMMU_DEBUG*/
+#ifdef IOMMU_DEBUG
+#define dbg(format, arg...) printk(KERN_ERR "[iommu]"format, ##arg);
+#else
+#define dbg(format, arg...)
+#endif
+
+#define SMMU_PHY_PTRS_PER_PTE (256)
+/*#define SMMU_PHY_PTRS_PER_PGD (4096)*/
+#define SMMU_PTRS_PER_PGD (4)
+#define SMMU_PTRS_PER_PMD (512)
+#define SMMU_PTRS_PER_PTE (512)
+#define SMMU_PAGE_SHIFT (12)
+
+#define PAGE_TABLE_ADDR_MASK (UL(0xFFFFFFF) << SMMU_PAGE_SHIFT)
+
+#define SMMU_PAGE_SIZE BIT(SMMU_PAGE_SHIFT)
+#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE-1))
+
+#define SMMU_PGDIR_SHIFT (30)
+#define SMMU_PGDIR_SIZE BIT(SMMU_PGDIR_SHIFT)
+#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE-1))
+
+#define SMMU_PMDIR_SHIFT (21)
+#define SMMU_PMDIR_SIZE BIT(SMMU_PMDIR_SHIFT)
+#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE-1))
+#define SMMU_PGD_TYPE (BIT(0) | BIT(1))
+#define SMMU_PMD_TYPE (BIT(0) | BIT(1))
+#define SMMU_PTE_TYPE (BIT(0) | BIT(1))
+
+#define SMMU_PGD_NS BIT(63)
+#define SMMU_PMD_NS BIT(63)
+#define SMMU_PTE_NS BIT(5)
+
+#define SMMU_PTE_PXN BIT(53) /* Privileged XN */
+#define SMMU_PTE_UXN BIT(54) /* User XN */
+#define SMMU_PTE_USER BIT(6) /* AP[1] */
+#define SMMU_PTE_RDONLY BIT(7) /* AP[2] */
+#define SMMU_PTE_SHARED (BIT(8) | BIT(9)) /* SH[1:0], inner shareable */
+#define SMMU_PTE_AF BIT(10) /* Access Flag */
+#define SMMU_PTE_NG BIT(11) /* nG */
+#define SMMU_PTE_ATTRINDX(t) ((t) << 2)
+/*
+ * Memory types available.
+ * USED BY A7
+ */
+#define HISI_MT_NORMAL 0
+#define HISI_MT_NORMAL_CACHE 4
+#define HISI_MT_NORMAL_NC 5
+#define HISI_MT_DEVICE_nGnRE 6
+
+
+#define SMMU_PAGE_DEFAULT (SMMU_PTE_TYPE | SMMU_PTE_AF | SMMU_PTE_SHARED)
+
+#define SMMU_PROT_DEVICE_nGnRE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
+ SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_DEVICE_nGnRE))
+#define SMMU_PROT_NORMAL_CACHE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
+ SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL_CACHE))
+#define SMMU_PROT_NORMAL_NC (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
+ SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL_NC))
+#define SMMU_PROT_NORMAL (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
+ SMMU_PTE_UXN | SMMU_PTE_ATTRINDX(HISI_MT_NORMAL))
+
+#define SMMU_PAGE_READWRITE (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \
+ SMMU_PTE_NG | SMMU_PTE_PXN | SMMU_PTE_UXN)
+#define SMMU_PAGE_READONLY (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \
+ SMMU_PTE_RDONLY | SMMU_PTE_NG | SMMU_PTE_PXN | SMMU_PTE_UXN)
+#define SMMU_PAGE_READONLY_EXEC (SMMU_PAGE_DEFAULT | SMMU_PTE_USER | \
+ SMMU_PTE_NG)
+
+#define smmu_pte_index(addr) (((addr) >> SMMU_PAGE_SHIFT) & (SMMU_PTRS_PER_PTE - 1))
+#define smmu_pmd_index(addr) (((addr) >> SMMU_PMDIR_SHIFT) & (SMMU_PTRS_PER_PMD - 1))
+#define smmu_pgd_index(addr) (((addr) >> SMMU_PGDIR_SHIFT) & (SMMU_PTRS_PER_PGD - 1))
+#define SMMU_PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE)
+
+typedef u64 smmu_pgd_t;
+typedef u64 smmu_pmd_t;
+typedef u64 smmu_pte_t;
+
+/*smmu device object*/
+struct hisi_smmu_device_lpae {
+ struct device *dev ;
+ struct list_head domain_list;
+ unsigned int ref_count;
+ spinlock_t lock;
+ unsigned long va_pgtable_addr;
+ phys_addr_t smmu_phy_pgtable_addr;
+ smmu_pgd_t *smmu_pgd;
+};
+
+struct hisi_map_tile_position_lpae {
+ struct scatterlist *sg ;
+ unsigned long offset;
+};
+
+extern struct hisi_smmu_device_lpae *hisi_smmu_dev;
+
+static inline unsigned int smmu_pgd_none_lpae(smmu_pgd_t pgd) {
+ return !(pgd ? pgd : 0);
+}
+
+static inline unsigned int smmu_pmd_none_lpae(smmu_pmd_t pmd) {
+ return !(pmd ? pmd : 0);
+}
+
+static inline unsigned int smmu_pte_none_lpae(smmu_pte_t pte) {
+ return !(pte ? pte : 0);
+}
+
+static inline unsigned int pte_is_valid_lpae(smmu_pte_t *ptep) {
+ return (unsigned int)((*(ptep)&SMMU_PTE_TYPE) ? 1 : 0);
+}
+
+/* Find an entry in the second-level page table.. */
+static inline void *smmu_pmd_page_vaddr_lpae(smmu_pmd_t *pgd)
+{
+ return phys_to_virt(*pgd & PAGE_TABLE_ADDR_MASK);
+}
+
+/* Find an entry in the third-level page table.. */
+static inline void *smmu_pte_page_vaddr_lpae(smmu_pmd_t *pmd)
+{
+ return phys_to_virt(*pmd & PAGE_TABLE_ADDR_MASK);
+}
+
+
+/*fill the pgd entry, pgd value must be 64bit */
+static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)
+{
+ *pgdp = pgd;
+ dsb(ishst);
+ isb();
+}
+
+/*fill the pmd entry, pgd value must be 64bit */
+static inline void smmu_set_pmd_lpae(smmu_pgd_t *pmdp, u64 pmd)
+{
+ dbg("smmu_set_pmd_lpae: pmd = 0x%lx \n", pmd);
+ *pmdp = pmd;
+ dsb(ishst);
+ isb();
+}
+
+static inline void smmu_pmd_populate_lpae(smmu_pmd_t *pmdp, pgtable_t ptep, pgdval_t prot)
+{
+ smmu_set_pmd_lpae(pmdp, (u64)(page_to_phys(ptep) | prot));
+}
+
+static inline void smmu_pgd_populate_lpae(smmu_pgd_t *pgdp, pgtable_t pmdp, pgdval_t prot)
+{
+ smmu_set_pgd_lpae(pgdp, (u64)(page_to_phys(pmdp) | prot));
+}
+
+static inline unsigned long smmu_pgd_addr_end_lpae(unsigned long addr, unsigned long end)
+{
+ unsigned long boundary = (addr + SMMU_PGDIR_SIZE) & SMMU_PGDIR_MASK;
+
+ return (boundary - 1 < end - 1) ? boundary : end;
+}
+
+static inline unsigned long smmu_pmd_addr_end_lpae(unsigned long addr, unsigned long end)
+{
+ unsigned long boundary = (addr + SMMU_PMDIR_SIZE) & SMMU_PMDIR_MASK;
+
+ return (boundary - 1 < end - 1) ? boundary : end;
+}
+
+int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
+ unsigned long iova, phys_addr_t paddr,
+ size_t size, int prot);
+
+unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
+ unsigned long iova, size_t size);
+
+#endif
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
new file mode 100644
index 000000000000..0ccd5c9ffeb1
--- /dev/null
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -0,0 +1,849 @@
+
+/*
+ * hisi_smmu_lpae.c -- 3 layer pagetable
+ *
+ * Copyright (c) 2014 Huawei Technologies CO., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iommu.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/sizes.h>
+#include <linux/spinlock.h>
+#include <asm/pgalloc.h>
+#include <linux/debugfs.h>
+#include <linux/hisi/hisi-iommu.h>
+#include <linux/uaccess.h>
+#include <linux/bitops.h>
+#include "hisi_smmu.h"
+
+struct hisi_smmu_device_lpae *hisi_smmu_dev;
+
+/*transfer 64bit pte table pointer to struct page*/
+static pgtable_t smmu_pgd_to_pte_lpae(unsigned int ppte_table)
+{
+ unsigned long page_table_addr;
+
+ if (!ppte_table) {
+ dbg("error: the pointer of pte_table is NULL\n");
+ return NULL;
+ }
+ page_table_addr = (unsigned long)ppte_table;
+ return phys_to_page(page_table_addr);
+}
+
+/*transfer 64bit pte table pointer to struct page*/
+static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table)
+{
+ struct page *table = NULL;
+
+ if (!ppte_table) {
+ dbg("error: the pointer of pte_table is NULL\n");
+ return NULL;
+ }
+ table = phys_to_page(ppte_table);
+ return table;
+}
+
+static int get_domain_data_lpae(struct device_node *np,
+ struct iommu_domain_data *data)
+{
+ unsigned long long align;
+ struct device_node *node = NULL;
+ int ret = 0;
+
+ data->phy_pgd_base = hisi_smmu_dev->smmu_phy_pgtable_addr;
+ if (np) {
+ node = of_find_node_by_name(np, "iommu_info");
+ if (!node) {
+ dbg("find iommu_info node error\n");
+ return -ENODEV;
+ }
+ ret = of_property_read_u32(node, "start-addr",
+ &data->iova_start);
+ if (ret) {
+ dbg("read iova start address error\n");
+ goto read_error;
+ }
+ ret = of_property_read_u32(node, "size", &data->iova_size);
+ if (ret) {
+ dbg("read iova size error\n");
+ goto read_error;
+ }
+ ret = of_property_read_u64(node, "iova-align", &align);
+ if (!ret)
+ data->iova_align = (unsigned long)align;
+ else
+ data->iova_align = SZ_256K;
+
+ pr_err("%s:start_addr 0x%x, size 0x%x align 0x%lx\n",
+ __func__, data->iova_start,
+ data->iova_size, data->iova_align);
+ }
+
+ return 0;
+
+read_error:
+ return ret;
+}
+
+static struct iommu_domain
+*hisi_smmu_domain_alloc_lpae(unsigned iommu_domain_type)
+{
+ struct iommu_domain *domain;
+
+ if (iommu_domain_type != IOMMU_DOMAIN_UNMANAGED)
+ return NULL;
+
+ domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+ if (!domain) {
+ pr_err("%s: fail to kzalloc %lu bytes\n",
+ __func__, sizeof(*domain));
+ }
+
+ return domain;
+}
+
+
+static void hisi_smmu_flush_pgtable_lpae(void *addr, size_t size)
+{
+ __flush_dcache_area(addr, size);
+}
+
+static void hisi_smmu_free_ptes_lpae(smmu_pgd_t pmd)
+{
+ pgtable_t table = smmu_pgd_to_pte_lpae(pmd);
+
+ if (!table) {
+ dbg("pte table is null\n");
+ return;
+ }
+ __free_page(table);
+ smmu_set_pmd_lpae(&pmd, 0);
+}
+
+
+static void hisi_smmu_free_pmds_lpae(smmu_pgd_t pgd)
+{
+ pgtable_t table = smmu_pmd_to_pte_lpae(pgd);
+
+ if (!table) {
+ dbg("pte table is null\n");
+ return;
+ }
+ __free_page(table);
+ smmu_set_pgd_lpae(&pgd, 0);
+}
+
+static void hisi_smmu_free_pgtables_lpae(unsigned long *page_table_addr)
+{
+ int i, j;
+ smmu_pgd_t *pgd;
+ smmu_pmd_t *pmd;
+ unsigned long flags;
+
+ pgd = (smmu_pgd_t *)page_table_addr;
+ pmd = (smmu_pmd_t *)page_table_addr;
+
+ spin_lock_irqsave(&hisi_smmu_dev->lock, flags);
+ for (i = 0; i < SMMU_PTRS_PER_PGD; ++i) {
+ if ((smmu_pgd_none_lpae(*pgd)) & (smmu_pmd_none_lpae(*pmd)))
+ continue;
+ for (j = 0; j < SMMU_PTRS_PER_PMD; ++j) {
+ hisi_smmu_free_pmds_lpae(*pgd);
+ pmd++;
+ }
+ hisi_smmu_free_ptes_lpae(*pmd);
+ pgd++;
+ }
+ memset((void *)page_table_addr, 0, PAGE_SIZE);
+ spin_unlock_irqrestore(&hisi_smmu_dev->lock, flags);
+}
+
+static void hisi_smmu_domain_free_lpae(struct iommu_domain *domain)
+{
+ if (list_empty(&hisi_smmu_dev->domain_list))
+ hisi_smmu_free_pgtables_lpae((unsigned long *)
+ hisi_smmu_dev->va_pgtable_addr);
+
+ kfree(domain);
+
+}
+
+static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
+ unsigned long addr, unsigned long end,
+ unsigned long pfn, u64 prot, unsigned long *flags)
+{
+ smmu_pte_t *pte, *start;
+ pgtable_t table;
+ u64 pteval = SMMU_PTE_TYPE;
+
+ if (!smmu_pmd_none_lpae(*ppmd))
+ goto pte_ready;
+
+ /* Allocate a new set of tables */
+ spin_unlock_irqrestore(&hisi_smmu_dev->lock, *flags);
+ table = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_DMA);
+ spin_lock_irqsave(&hisi_smmu_dev->lock, *flags);
+ if (!table) {
+ dbg("%s: alloc page fail\n", __func__);
+ return -ENOMEM;
+ }
+
+ if (smmu_pmd_none_lpae(*ppmd)) {
+ hisi_smmu_flush_pgtable_lpae(page_address(table),
+ SMMU_PAGE_SIZE);
+ smmu_pmd_populate_lpae(ppmd, table, SMMU_PMD_TYPE|SMMU_PMD_NS);
+ hisi_smmu_flush_pgtable_lpae(ppmd, sizeof(*ppmd));
+ } else
+ __free_page(table);
+
+pte_ready:
+ if (prot & IOMMU_SEC)
+ *ppmd &= (~SMMU_PMD_NS);
+
+ start = (smmu_pte_t *)smmu_pte_page_vaddr_lpae(ppmd)
+ + smmu_pte_index(addr);
+ pte = start;
+ if (!prot) {
+ pteval |= SMMU_PROT_NORMAL;
+ pteval |= SMMU_PTE_NS;
+ } else {
+ if (prot & IOMMU_DEVICE) {
+ pteval |= SMMU_PROT_DEVICE_nGnRE;
+ } else {
+ if (prot & IOMMU_CACHE)
+ pteval |= SMMU_PROT_NORMAL_CACHE;
+ else
+ pteval |= SMMU_PROT_NORMAL_NC;
+
+ if ((prot & IOMMU_READ) && (prot & IOMMU_WRITE))
+ pteval |= SMMU_PAGE_READWRITE;
+ else if ((prot & IOMMU_READ) && !(prot & IOMMU_WRITE))
+ pteval |= SMMU_PAGE_READONLY;
+ else
+ WARN_ON("you do not set read attribute!");
+
+ if (prot & IOMMU_EXEC) {
+ pteval |= SMMU_PAGE_READONLY_EXEC;
+ pteval &= ~(SMMU_PTE_PXN | SMMU_PTE_UXN);
+ }
+ }
+ if (prot & IOMMU_SEC)
+ pteval &= (~SMMU_PTE_NS);
+ else
+ pteval |= SMMU_PTE_NS;
+ }
+
+ do {
+ if (!pte_is_valid_lpae(pte))
+ *pte = (u64)(__pfn_to_phys(pfn)|pteval);
+ else
+ WARN_ONCE(1, "map to same VA more times!\n");
+ pte++;
+ pfn++;
+ addr += SMMU_PAGE_SIZE;
+ } while (addr < end);
+
+ hisi_smmu_flush_pgtable_lpae(start, sizeof(*pte) * (pte - start));
+ return 0;
+}
+
+static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
+ unsigned long addr, unsigned long end,
+ unsigned long paddr, int prot, unsigned long *flags)
+{
+ int ret = 0;
+ smmu_pmd_t *ppmd, *start;
+ u64 next;
+ pgtable_t table;
+
+ if (!smmu_pgd_none_lpae(*ppgd))
+ goto pmd_ready;
+
+ /* Allocate a new set of tables */
+ spin_unlock_irqrestore(&hisi_smmu_dev->lock, *flags);
+ table = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_DMA);
+ spin_lock_irqsave(&hisi_smmu_dev->lock, *flags);
+ if (!table) {
+ dbg("%s: alloc page fail\n", __func__);
+ return -ENOMEM;
+ }
+
+ if (smmu_pgd_none_lpae(*ppgd)) {
+ hisi_smmu_flush_pgtable_lpae(page_address(table),
+ SMMU_PAGE_SIZE);
+ smmu_pgd_populate_lpae(ppgd, table, SMMU_PGD_TYPE|SMMU_PGD_NS);
+ hisi_smmu_flush_pgtable_lpae(ppgd, sizeof(*ppgd));
+ } else
+ __free_page(table);
+
+pmd_ready:
+ if (prot & IOMMU_SEC)
+ *ppgd &= (~SMMU_PGD_NS);
+ start = (smmu_pmd_t *)smmu_pmd_page_vaddr_lpae(ppgd)
+ + smmu_pmd_index(addr);
+ ppmd = start;
+
+ do {
+ next = smmu_pmd_addr_end_lpae(addr, end);
+ ret = hisi_smmu_alloc_init_pte_lpae(ppmd,
+ addr, next, __phys_to_pfn(paddr), prot, flags);
+ if (ret)
+ goto error;
+ paddr += (next - addr);
+ addr = next;
+ } while (ppmd++, addr < end);
+error:
+ return ret;
+}
+
+int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
+ unsigned long iova, phys_addr_t paddr,
+ size_t size, int prot)
+{
+ int ret;
+ unsigned long end;
+ unsigned long next;
+ unsigned long flags;
+ smmu_pgd_t *pgd = (smmu_pgd_t *)hisi_smmu_dev->va_pgtable_addr;
+
+ if (!pgd) {
+ dbg("pgd is null\n");
+ return -EINVAL;
+ }
+ iova = ALIGN(iova, SMMU_PAGE_SIZE);
+ size = ALIGN(size, SMMU_PAGE_SIZE);
+ spin_lock_irqsave(&hisi_smmu_dev->lock, flags);
+ pgd += smmu_pgd_index(iova);
+ end = iova + size;
+ do {
+ next = smmu_pgd_addr_end_lpae(iova, end);
+ ret = hisi_smmu_alloc_init_pmd_lpae(pgd,
+ iova, next, paddr, prot, &flags);
+ if (ret)
+ goto out_unlock;
+ paddr += next - iova;
+ iova = next;
+ } while (pgd++, iova < end);
+out_unlock:
+ spin_unlock_irqrestore(&hisi_smmu_dev->lock, flags);
+ return ret;
+}
+
+static int hisi_smmu_map_lpae(struct iommu_domain *domain,
+ unsigned long iova,
+ phys_addr_t paddr, size_t size,
+ int prot)
+{
+ unsigned long max_iova;
+ struct iommu_domain_data *data;
+
+ if (!domain) {
+ dbg("domain is null\n");
+ return -ENODEV;
+ }
+ data = domain->priv;
+ max_iova = data->iova_start + data->iova_size;
+ if (iova < data->iova_start) {
+ dbg("iova failed: iova = 0x%lx, start = 0x%8x\n",
+ iova, data->iova_start);
+ goto error;
+ }
+ if ((iova+size) > max_iova) {
+ dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
+ iova+size, max_iova);
+ goto error;
+ }
+ return hisi_smmu_handle_mapping_lpae(domain, iova, paddr, size, prot);
+error:
+ dbg("iova is not in this range\n");
+ return -EINVAL;
+}
+
+static unsigned int hisi_smmu_clear_pte_lpae(smmu_pgd_t *pmdp,
+ unsigned int iova, unsigned int end)
+{
+ smmu_pte_t *ptep = NULL;
+ smmu_pte_t *ppte = NULL;
+ unsigned int size = end - iova;
+
+ ptep = smmu_pte_page_vaddr_lpae(pmdp);
+ ppte = ptep + smmu_pte_index(iova);
+
+ if (!!size)
+ memset(ppte, 0x0, (size / SMMU_PAGE_SIZE) * sizeof(*ppte));
+
+ return size;
+}
+
+static unsigned int hisi_smmu_clear_pmd_lpae(smmu_pgd_t *pgdp,
+ unsigned int iova, unsigned int end)
+{
+ smmu_pmd_t *pmdp = NULL;
+ smmu_pmd_t *ppmd = NULL;
+ unsigned int next = 0;
+ unsigned int size = end - iova;
+
+ pmdp = smmu_pmd_page_vaddr_lpae(pgdp);
+ ppmd = pmdp + smmu_pmd_index(iova);
+ do {
+ next = smmu_pmd_addr_end_lpae(iova, end);
+ hisi_smmu_clear_pte_lpae(ppmd, iova, next);
+ iova = next;
+ dbg("%s: iova=0x%lx, end=0x%lx\n", __func__, iova, end);
+ } while (ppmd++, iova < end);
+
+ return size;
+}
+
+unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
+ unsigned long iova, size_t size)
+{
+ smmu_pgd_t *pgdp = NULL;
+ unsigned int end = 0;
+ unsigned int next = 0;
+ unsigned int unmap_size = 0;
+ unsigned long flags;
+
+ iova = SMMU_PAGE_ALIGN(iova);
+ size = SMMU_PAGE_ALIGN(size);
+ pgdp = (smmu_pgd_t *)hisi_smmu_dev->va_pgtable_addr;
+ end = iova + size;
+ dbg("%s:end=0x%x\n", __func__, end);
+ pgdp += smmu_pgd_index(iova);
+ spin_lock_irqsave(&hisi_smmu_dev->lock, flags);
+ do {
+ next = smmu_pgd_addr_end_lpae(iova, end);
+ unmap_size += hisi_smmu_clear_pmd_lpae(pgdp, iova, next);
+ iova = next;
+ dbg("%s: pgdp=%p, iova=0x%lx\n", __func__, pgdp, iova);
+ } while (pgdp++, iova < end);
+
+ spin_unlock_irqrestore(&hisi_smmu_dev->lock, flags);
+ return unmap_size;
+}
+
+static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
+ unsigned long iova, size_t size)
+{
+ unsigned long max_iova;
+ unsigned int ret;
+ struct iommu_domain_data *data;
+
+ if (!domain) {
+ dbg("domain is null\n");
+ return -ENODEV;
+ }
+ data = domain->priv;
+ /*caculate the max io virtual address */
+ max_iova = data->iova_start + data->iova_size;
+ /*check the iova */
+ if (iova < data->iova_start)
+ goto error;
+ if ((iova+size) > max_iova) {
+ dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
+ iova+size, max_iova);
+ goto error;
+ }
+ /*unmapping the range of iova*/
+ ret = hisi_smmu_handle_unmapping_lpae(domain, iova, size);
+ if (ret == size) {
+ dbg("%s:unmap size:0x%x\n", __func__, (unsigned int)size);
+ return size;
+ } else {
+ return 0;
+ }
+error:
+ dbg("%s:the range of io address is wrong\n", __func__);
+ return -EINVAL;
+}
+
+static phys_addr_t hisi_smmu_iova_to_phys_lpae(
+ struct iommu_domain *domain, dma_addr_t iova)
+{
+ smmu_pgd_t *pgdp, pgd;
+ smmu_pmd_t pmd;
+ smmu_pte_t pte;
+
+ pgdp = (smmu_pgd_t *)hisi_smmu_dev->va_pgtable_addr;
+ if (!pgdp)
+ return 0;
+
+ pgd = *(pgdp + smmu_pgd_index(iova));
+ if (smmu_pgd_none_lpae(pgd))
+ return 0;
+
+ pmd = *((smmu_pmd_t *)smmu_pmd_page_vaddr_lpae(&pgd) +
+ smmu_pmd_index(iova));
+ if (smmu_pmd_none_lpae(pmd))
+ return 0;
+
+ pte = *((u64 *)smmu_pte_page_vaddr_lpae(&pmd) + smmu_pte_index(iova));
+ if (smmu_pte_none_lpae(pte))
+ return 0;
+
+ return __pfn_to_phys(pte_pfn(__pte(pte))) | (iova & ~SMMU_PAGE_MASK);
+}
+
+static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev)
+{
+ struct device_node *np = dev->of_node;
+ int ret = 0;
+ struct iommu_domain_data *iommu_info = NULL;
+
+ iommu_info = kzalloc(sizeof(struct iommu_domain_data), GFP_KERNEL);
+ if (!iommu_info) {
+ dbg("alloc iommu_domain_data fail\n");
+ return -EINVAL;
+ }
+ list_add(&iommu_info->list, &hisi_smmu_dev->domain_list);
+ domain->priv = iommu_info;
+ ret = get_domain_data_lpae(np, domain->priv);
+ return ret;
+}
+
+static void hisi_detach_dev_lpae(struct iommu_domain *domain,
+ struct device *dev)
+{
+ struct iommu_domain_data *data;
+
+ data = (struct iommu_domain_data *)domain->priv;
+ if (data) {
+ list_del(&data->list);
+ domain->priv = NULL;
+ kfree(data);
+ } else {
+ dbg("%s:error! data entry has been delected\n", __func__);
+ }
+}
+
+static dma_addr_t get_phys_addr_lpae(struct scatterlist *sg)
+{
+ dma_addr_t dma_addr = sg_dma_address(sg);
+
+ if (!dma_addr)
+ dma_addr = sg_phys(sg);
+ return dma_addr;
+}
+
+int iommu_map_tile(struct iommu_domain *domain, unsigned long iova,
+ struct scatterlist *sg, size_t size, int prot,
+ struct tile_format *format)
+{
+ if (unlikely(!(domain->ops->map_tile)))
+ return -ENODEV;
+
+ BUG_ON(iova & (~PAGE_MASK));
+
+ return domain->ops->map_tile(domain, iova, sg, size, prot, format);
+}
+
+int iommu_unmap_tile(struct iommu_domain *domain, unsigned long iova,
+ size_t size)
+{
+ if (unlikely(!(domain->ops->unmap_tile)))
+ return -ENODEV;
+
+ BUG_ON(iova & (~PAGE_MASK));
+
+ return domain->ops->unmap_tile(domain, iova, size);
+}
+
+/*
+ *iova: the start address for tile mapping
+ *size: the physical memory size
+ *sg: the node of scatter list where are the start node of physical memory
+ *sg_offset:the physical memory offset in the sg node ,where is the start
+ position of physical memory
+ *port: the pape property of virtual memory
+ * this function complete one row mapping.
+ */
+static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
+ iova, size_t size, struct scatterlist *sg, size_t sg_offset,
+ struct hisi_map_tile_position_lpae *map_position,
+ unsigned int prot){
+
+ unsigned long map_size; /*the memory size that will be mapped*/
+ unsigned long phys_addr;
+ unsigned long mapped_size = 0; /*memory size that has been mapped*/
+ int ret;
+
+ while (1) {
+ /*
+ *get the remain memory,if current sg node is not enough memory,
+ *we map the remain memory firstly.
+ */
+ map_size = size - mapped_size;
+ if (map_size > (sg->length - sg_offset))
+ map_size = (sg->length - sg_offset);
+
+ /*get the start physical address*/
+ phys_addr = (unsigned long)get_phys_addr_lpae(sg) + sg_offset;
+ ret = hisi_smmu_map_lpae(domain,
+ iova + mapped_size, phys_addr, map_size, prot);
+ if (ret) {
+ dbg("[%s] hisi_smmu_map failed!\n", __func__);
+ break;
+ }
+ /*update mapped memory size*/
+ mapped_size += map_size;
+ /*
+ * if finished mapping,
+ * we update the memory offset of current node and
+ * save the memory position. otherwise we clean the sg_offset
+ * to zero and get next sg node.
+ */
+ if (mapped_size < size) {
+ sg_offset = 0;
+ sg = sg_next(sg);
+ if (!sg) {
+ dbg("[%s] phy memory not enough\n", __func__);
+ break;
+ }
+ } else {
+ sg_offset += map_size;
+ /*if physcial memory of this node is exhausted,
+ * we choose next node
+ */
+ if (sg_offset == sg->length) {
+ sg_offset = 0;
+ sg = sg_next(sg);
+ }
+ break;
+ }
+ }
+ /*save current position*/
+ map_position->sg = sg;
+ map_position->offset = sg_offset;
+
+ return mapped_size;
+}
+
+/*
+ *domain:the iommu domain for mapping
+ *iova:the start virtual address
+ *sg: the scatter list of physical memory
+ *size:the total size of all virtual memory
+ *port:the property of page table of virtual memory
+ *format:the parameter of tile mapping
+ *this function map physical memory in tile mode
+ */
+static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
+ unsigned long iova,
+ struct scatterlist *sg, size_t size, int prot,
+ struct tile_format *format){
+
+ unsigned int phys_length;
+ struct scatterlist *sg_node;
+ unsigned int row_number, row;
+ unsigned int size_virt, size_phys;
+ unsigned int sg_offset;
+ int ret = size;
+ unsigned int mapped_size, header_size;
+ struct hisi_map_tile_position_lpae map_position;
+
+ /* calculate the whole length of phys mem */
+ for (phys_length = 0, sg_node = sg; sg_node; sg_node = sg_next(sg_node))
+ phys_length += ALIGN(sg_node->length, PAGE_SIZE);
+
+ header_size = format->header_size;
+
+ /* calculate the number of raws*/
+ row_number = ((phys_length - header_size) >> PAGE_SHIFT)
+ / format->phys_page_line;
+ dbg("phys_length: 0x%x, rows: 0x%x, header_size: 0x%x\n",
+ phys_length, row_number, header_size);
+
+ /*caculate the need physical memory and virtual memory for one row*/
+ size_phys = (format->phys_page_line * PAGE_SIZE);
+ size_virt = (format->virt_page_line * PAGE_SIZE);
+
+ sg_offset = 0;
+ sg_node = sg;
+
+ /*set start position*/
+ map_position.sg = sg;
+ map_position.offset = 0;
+
+ /*map header*/
+ if (header_size) {
+ mapped_size = hisi_map_tile_row_lpae(domain, iova,
+ header_size, sg_node,
+ sg_offset, &map_position,
+ prot);
+ if (mapped_size != header_size) {
+ WARN(1, "map head fail\n");
+ ret = -EINVAL;
+ goto error;
+ }
+ iova += ALIGN(header_size, size_virt);
+ }
+ /* map row by row */
+ for (row = 0; row < row_number; row++) {
+ /* get physical memory position */
+ if (map_position.sg) {
+ sg_node = map_position.sg;
+ sg_offset = map_position.offset;
+ } else {
+ dbg("[%s]:physical memory is not enough\n", __func__);
+ break;
+ }
+ /* map one row*/
+ mapped_size = hisi_map_tile_row_lpae(domain,
+ iova + (size_virt * row),
+ size_phys, sg_node, sg_offset,
+ &map_position, prot);
+ if (mapped_size != size_phys) {
+ WARN(1, "hisi_map_tile_row failed!\n");
+ ret = -EINVAL;
+ break;
+ }
+ };
+error:
+ return ret;
+}
+
+static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
+ unsigned long iova, size_t size)
+{
+ return hisi_smmu_unmap_lpae(domain, iova, size);
+}
+
+size_t hisi_iommu_map_sg_lpae(struct iommu_domain *domain, unsigned long iova,
+ struct scatterlist *sg, unsigned int nents, int prot)
+{
+ struct scatterlist *s;
+ size_t mapped = 0;
+ unsigned int i, min_pagesz;
+ int ret;
+
+ if (domain->ops->pgsize_bitmap == 0UL)
+ return 0;
+
+ min_pagesz = (unsigned int)1 << __ffs(domain->ops->pgsize_bitmap);
+
+ for_each_sg(sg, s, nents, i) {
+ phys_addr_t phys = page_to_phys(sg_page(s)) + s->offset;
+
+ /*
+ * We are mapping on IOMMU page boundaries, so offset within
+ * the page must be 0. However, the IOMMU may support pages
+ * smaller than PAGE_SIZE, so s->offset may still represent
+ * an offset of that boundary within the CPU page.
+ */
+ if (!IS_ALIGNED(s->offset, min_pagesz))
+ goto out_err;
+
+ ret = hisi_smmu_map_lpae(domain, iova + mapped, phys,
+ (size_t)s->length, prot);
+ if (ret)
+ goto out_err;
+
+ mapped += s->length;
+ }
+
+ return mapped;
+
+out_err:
+ /* undo mappings already done */
+ hisi_smmu_unmap_lpae(domain, iova, mapped);
+
+ return 0;
+}
+
+static struct iommu_ops hisi_smmu_ops = {
+ .domain_alloc = hisi_smmu_domain_alloc_lpae,
+ .domain_free = hisi_smmu_domain_free_lpae,
+ .map = hisi_smmu_map_lpae,
+ .unmap = hisi_smmu_unmap_lpae,
+ .map_sg = hisi_iommu_map_sg_lpae,
+ .attach_dev = hisi_attach_dev_lpae,
+ .detach_dev = hisi_detach_dev_lpae,
+ .iova_to_phys = hisi_smmu_iova_to_phys_lpae,
+ .pgsize_bitmap = SMMU_PAGE_SIZE,
+ .map_tile = hisi_smmu_map_tile_lpae,
+ .unmap_tile = hisi_smmu_unmap_tile_lpae,
+};
+
+static int hisi_smmu_probe_lpae(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ dbg("enter %s\n", __func__);
+ hisi_smmu_dev = devm_kzalloc(dev,
+ sizeof(struct hisi_smmu_device_lpae), GFP_KERNEL);
+
+ hisi_smmu_dev->smmu_pgd = devm_kzalloc(dev, SZ_64, GFP_KERNEL | __GFP_DMA);
+ if (!hisi_smmu_dev)
+ goto smmu_device_error;
+ hisi_smmu_dev->dev = dev;
+ INIT_LIST_HEAD(&hisi_smmu_dev->domain_list);
+ spin_lock_init(&hisi_smmu_dev->lock);
+
+ hisi_smmu_dev->smmu_pgd = (smmu_pgd_t *)(ALIGN((unsigned long)(hisi_smmu_dev->smmu_pgd), SZ_32));
+
+ hisi_smmu_dev->smmu_phy_pgtable_addr =
+ virt_to_phys(hisi_smmu_dev->smmu_pgd);
+ printk(KERN_ERR "%s, smmu_phy_pgtable_addr is = %llx\n", __func__, hisi_smmu_dev->smmu_phy_pgtable_addr);
+
+ hisi_smmu_dev->va_pgtable_addr = (unsigned long)(hisi_smmu_dev->smmu_pgd);
+ bus_set_iommu(&platform_bus_type, &hisi_smmu_ops);
+ return 0;
+
+smmu_device_error:
+ return -ENOMEM;
+}
+
+static int hisi_smmu_remove_lpae(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id hisi_smmu_of_match_lpae[] = {
+ { .compatible = "hisi,hisi-smmu-lpae"},
+ { },
+};
+MODULE_DEVICE_TABLE(of, hisi_smmu_of_match_lpae);
+
+static struct platform_driver hisi_smmu_driver_lpae = {
+ .driver = {
+ .owner = THIS_MODULE,
+ .name = "hisi-smmu-lpae",
+ .of_match_table = of_match_ptr(hisi_smmu_of_match_lpae),
+ },
+ .probe = hisi_smmu_probe_lpae,
+ .remove = hisi_smmu_remove_lpae,
+};
+
+static int __init hisi_smmu_init_lpae(void)
+{
+ int ret = 0;
+
+ ret = platform_driver_register(&hisi_smmu_driver_lpae);
+ return ret;
+}
+
+static void __exit hisi_smmu_exit_lpae(void)
+{
+ return platform_driver_unregister(&hisi_smmu_driver_lpae);
+}
+
+subsys_initcall(hisi_smmu_init_lpae);
+module_exit(hisi_smmu_exit_lpae);
+
+MODULE_DESCRIPTION("IOMMU API for HI3660 architected SMMU implementations");
+MODULE_AUTHOR("huawei hisilicon company");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/hisi/hisi-iommu.h b/include/linux/hisi/hisi-iommu.h
new file mode 100644
index 000000000000..00dd5e97db59
--- /dev/null
+++ b/include/linux/hisi/hisi-iommu.h
@@ -0,0 +1,13 @@
+#ifndef _HI36XX_SMMU_H
+#define _HI36XX_SMMU_H
+
+#include <linux/types.h>
+struct iommu_domain_data {
+ unsigned int iova_start;
+ unsigned int iova_size;
+ phys_addr_t phy_pgd_base;
+ unsigned long iova_align;
+ struct list_head list;
+};
+
+#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 02/16] iommu: hisilicon: remove default iommu_map_sg handler
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (2 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, Joerg Roedel, linux-kernel, devel
The code there is just a copy of the default iommu sg
handler. Well, callback fops was removed, as all iommu
drivers are doing the same.
Fixes: d88e61faad52 ("iommu: Remove the ->map_sg indirection")
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 42 -----------------------
1 file changed, 42 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 0ccd5c9ffeb1..c5c266fb1c0b 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -720,48 +720,7 @@ static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
unsigned long iova, size_t size)
{
return hisi_smmu_unmap_lpae(domain, iova, size);
-}
-size_t hisi_iommu_map_sg_lpae(struct iommu_domain *domain, unsigned long iova,
- struct scatterlist *sg, unsigned int nents, int prot)
-{
- struct scatterlist *s;
- size_t mapped = 0;
- unsigned int i, min_pagesz;
- int ret;
-
- if (domain->ops->pgsize_bitmap == 0UL)
- return 0;
-
- min_pagesz = (unsigned int)1 << __ffs(domain->ops->pgsize_bitmap);
-
- for_each_sg(sg, s, nents, i) {
- phys_addr_t phys = page_to_phys(sg_page(s)) + s->offset;
-
- /*
- * We are mapping on IOMMU page boundaries, so offset within
- * the page must be 0. However, the IOMMU may support pages
- * smaller than PAGE_SIZE, so s->offset may still represent
- * an offset of that boundary within the CPU page.
- */
- if (!IS_ALIGNED(s->offset, min_pagesz))
- goto out_err;
-
- ret = hisi_smmu_map_lpae(domain, iova + mapped, phys,
- (size_t)s->length, prot);
- if (ret)
- goto out_err;
-
- mapped += s->length;
- }
-
- return mapped;
-
-out_err:
- /* undo mappings already done */
- hisi_smmu_unmap_lpae(domain, iova, mapped);
-
- return 0;
}
static struct iommu_ops hisi_smmu_ops = {
@@ -769,7 +728,6 @@ static struct iommu_ops hisi_smmu_ops = {
.domain_free = hisi_smmu_domain_free_lpae,
.map = hisi_smmu_map_lpae,
.unmap = hisi_smmu_unmap_lpae,
- .map_sg = hisi_iommu_map_sg_lpae,
.attach_dev = hisi_attach_dev_lpae,
.detach_dev = hisi_detach_dev_lpae,
.iova_to_phys = hisi_smmu_iova_to_phys_lpae,
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 03/16] iommu: hisilicon: map and unmap ops gained new arguments
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (3 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
As both map() and unmap() ops gained new arguments upstream,
update their headers accordingly.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index c5c266fb1c0b..b411ca97f2c2 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -346,7 +346,8 @@ int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
static int hisi_smmu_map_lpae(struct iommu_domain *domain,
unsigned long iova,
phys_addr_t paddr, size_t size,
- int prot)
+ int prot,
+ gfp_t gfp)
{
unsigned long max_iova;
struct iommu_domain_data *data;
@@ -437,7 +438,8 @@ unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
}
static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size)
+ unsigned long iova, size_t size,
+ struct iommu_iotlb_gather *iotlb_gather)
{
unsigned long max_iova;
unsigned int ret;
@@ -593,7 +595,7 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
/*get the start physical address*/
phys_addr = (unsigned long)get_phys_addr_lpae(sg) + sg_offset;
ret = hisi_smmu_map_lpae(domain,
- iova + mapped_size, phys_addr, map_size, prot);
+ iova + mapped_size, phys_addr, map_size, prot, GFP_KERNEL);
if (ret) {
dbg("[%s] hisi_smmu_map failed!\n", __func__);
break;
@@ -719,8 +721,7 @@ static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
unsigned long iova, size_t size)
{
- return hisi_smmu_unmap_lpae(domain, iova, size);
-
+ return hisi_smmu_unmap_lpae(domain, iova, size, NULL);
}
static struct iommu_ops hisi_smmu_ops = {
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 04/16] iommu: hisi_smmu_lpae: rebase it to work with upstream
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (4 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Currently, this driver OOPSes. It turns that this driver
needs to be updated in order to work with the current
iommu kAPI.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 2 +
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 81 ++++++++++++++++++++---
2 files changed, 74 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
index 4637244dba6b..bd3ee53735bd 100644
--- a/drivers/staging/hikey9xx/hisi_smmu.h
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -89,6 +89,8 @@ struct hisi_smmu_device_lpae {
unsigned long va_pgtable_addr;
phys_addr_t smmu_phy_pgtable_addr;
smmu_pgd_t *smmu_pgd;
+
+ struct iommu_device iommu;
};
struct hisi_map_tile_position_lpae {
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index b411ca97f2c2..5acde3ddbd99 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -724,30 +724,73 @@ static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
return hisi_smmu_unmap_lpae(domain, iova, size, NULL);
}
+static bool hisi_smmu_capable(enum iommu_cap cap)
+{
+ return false;
+}
+
+static int hisi_smmu_add_device(struct device *dev)
+{
+ struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev;
+ struct iommu_group *group;
+
+ if (iommu)
+ iommu_device_link(&iommu->iommu, dev);
+ else
+ return -ENODEV;
+
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group))
+ return PTR_ERR(group);
+
+ iommu_group_put(group);
+
+ return 0;
+}
+
+static void hisi_smmu_remove_device(struct device *dev)
+{
+ struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev;
+
+ if (iommu)
+ iommu_device_unlink(&iommu->iommu, dev);
+
+ iommu_group_remove_device(dev);
+}
+
static struct iommu_ops hisi_smmu_ops = {
+ .capable = hisi_smmu_capable,
.domain_alloc = hisi_smmu_domain_alloc_lpae,
.domain_free = hisi_smmu_domain_free_lpae,
+ .attach_dev = hisi_attach_dev_lpae,
+ .detach_dev = hisi_detach_dev_lpae,
.map = hisi_smmu_map_lpae,
.unmap = hisi_smmu_unmap_lpae,
- .attach_dev = hisi_attach_dev_lpae,
- .detach_dev = hisi_detach_dev_lpae,
.iova_to_phys = hisi_smmu_iova_to_phys_lpae,
+ .add_device = hisi_smmu_add_device,
+ .remove_device = hisi_smmu_remove_device,
+ .device_group = generic_device_group,
.pgsize_bitmap = SMMU_PAGE_SIZE,
- .map_tile = hisi_smmu_map_tile_lpae,
- .unmap_tile = hisi_smmu_unmap_tile_lpae,
+ .map_tile = hisi_smmu_map_tile_lpae,
+ .unmap_tile = hisi_smmu_unmap_tile_lpae,
};
static int hisi_smmu_probe_lpae(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ int ret;
dbg("enter %s\n", __func__);
hisi_smmu_dev = devm_kzalloc(dev,
sizeof(struct hisi_smmu_device_lpae), GFP_KERNEL);
- hisi_smmu_dev->smmu_pgd = devm_kzalloc(dev, SZ_64, GFP_KERNEL | __GFP_DMA);
- if (!hisi_smmu_dev)
+ hisi_smmu_dev->smmu_pgd = devm_kzalloc(dev, SZ_64,
+ GFP_KERNEL | __GFP_DMA);
+ if (!hisi_smmu_dev) {
+ ret = -ENOMEM;
goto smmu_device_error;
+ }
+
hisi_smmu_dev->dev = dev;
INIT_LIST_HEAD(&hisi_smmu_dev->domain_list);
spin_lock_init(&hisi_smmu_dev->lock);
@@ -756,18 +799,39 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
hisi_smmu_dev->smmu_phy_pgtable_addr =
virt_to_phys(hisi_smmu_dev->smmu_pgd);
- printk(KERN_ERR "%s, smmu_phy_pgtable_addr is = %llx\n", __func__, hisi_smmu_dev->smmu_phy_pgtable_addr);
+ dev_info(&pdev->dev, "%s, smmu_phy_pgtable_addr is = 0x%llx\n",
+ __func__, hisi_smmu_dev->smmu_phy_pgtable_addr);
hisi_smmu_dev->va_pgtable_addr = (unsigned long)(hisi_smmu_dev->smmu_pgd);
+
+ ret = iommu_device_sysfs_add(&hisi_smmu_dev->iommu, NULL, NULL,
+ "hisi-iommu");
+ if (ret)
+ goto fail_register;
+
+ iommu_device_set_ops(&hisi_smmu_dev->iommu, &hisi_smmu_ops);
+
+ ret = iommu_device_register(&hisi_smmu_dev->iommu);
+ if (ret) {
+ dev_info(&pdev->dev, "Could not register hisi-smmu\n");
+ goto fail_register;
+ }
+
bus_set_iommu(&platform_bus_type, &hisi_smmu_ops);
return 0;
+fail_register:
+ iommu_device_sysfs_remove(&hisi_smmu_dev->iommu);
+
smmu_device_error:
- return -ENOMEM;
+ return ret;
}
static int hisi_smmu_remove_lpae(struct platform_device *pdev)
{
+ iommu_device_unregister(&hisi_smmu_dev->iommu);
+ iommu_device_sysfs_remove(&hisi_smmu_dev->iommu);
+
return 0;
}
@@ -779,7 +843,6 @@ MODULE_DEVICE_TABLE(of, hisi_smmu_of_match_lpae);
static struct platform_driver hisi_smmu_driver_lpae = {
.driver = {
- .owner = THIS_MODULE,
.name = "hisi-smmu-lpae",
.of_match_table = of_match_ptr(hisi_smmu_of_match_lpae),
},
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 05/16] iommu: hisi_smmu: remove linux/hisi/hisi-iommu.h
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (5 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, Chenfeng, Suzhuangluan, linux-kernel,
devel
Just move its contents into drivers/iommu/hisi_smmu.h.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 8 ++++++++
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 1 -
include/linux/hisi/hisi-iommu.h | 13 -------------
3 files changed, 8 insertions(+), 14 deletions(-)
delete mode 100644 include/linux/hisi/hisi-iommu.h
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
index bd3ee53735bd..c84f854bf39f 100644
--- a/drivers/staging/hikey9xx/hisi_smmu.h
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -93,6 +93,14 @@ struct hisi_smmu_device_lpae {
struct iommu_device iommu;
};
+struct iommu_domain_data {
+ unsigned int iova_start;
+ unsigned int iova_size;
+ phys_addr_t phy_pgd_base;
+ unsigned long iova_align;
+ struct list_head list;
+};
+
struct hisi_map_tile_position_lpae {
struct scatterlist *sg ;
unsigned long offset;
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 5acde3ddbd99..fcaf97f92e7f 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -24,7 +24,6 @@
#include <linux/spinlock.h>
#include <asm/pgalloc.h>
#include <linux/debugfs.h>
-#include <linux/hisi/hisi-iommu.h>
#include <linux/uaccess.h>
#include <linux/bitops.h>
#include "hisi_smmu.h"
diff --git a/include/linux/hisi/hisi-iommu.h b/include/linux/hisi/hisi-iommu.h
deleted file mode 100644
index 00dd5e97db59..000000000000
--- a/include/linux/hisi/hisi-iommu.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef _HI36XX_SMMU_H
-#define _HI36XX_SMMU_H
-
-#include <linux/types.h>
-struct iommu_domain_data {
- unsigned int iova_start;
- unsigned int iova_size;
- phys_addr_t phy_pgd_base;
- unsigned long iova_align;
- struct list_head list;
-};
-
-#endif
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 06/16] iommu: hisilicon: cleanup its code style
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (6 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Fix most of the things complained by checkpatch on strict
mode:
- Replaced BUG_ON to WARN_ON;
- added SPDX headers;
- adjusted alignments;
- used --fix-inplace to solve other minor issues.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 40 ++--
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 243 +++++++++++-----------
2 files changed, 143 insertions(+), 140 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
index c84f854bf39f..b2d32ec6cb84 100644
--- a/drivers/staging/hikey9xx/hisi_smmu.h
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -1,9 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
#ifndef HISI_SMMU_H
#define HISI_SMMU_H
/*#define IOMMU_DEBUG*/
#ifdef IOMMU_DEBUG
-#define dbg(format, arg...) printk(KERN_ERR "[iommu]"format, ##arg);
+#define dbg(format, arg...) printk(KERN_ERR "[iommu]" format, ##arg)
#else
#define dbg(format, arg...)
#endif
@@ -18,15 +20,15 @@
#define PAGE_TABLE_ADDR_MASK (UL(0xFFFFFFF) << SMMU_PAGE_SHIFT)
#define SMMU_PAGE_SIZE BIT(SMMU_PAGE_SHIFT)
-#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE-1))
+#define SMMU_PAGE_MASK (~(SMMU_PAGE_SIZE - 1))
#define SMMU_PGDIR_SHIFT (30)
#define SMMU_PGDIR_SIZE BIT(SMMU_PGDIR_SHIFT)
-#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE-1))
+#define SMMU_PGDIR_MASK (~(SMMU_PGDIR_SIZE - 1))
#define SMMU_PMDIR_SHIFT (21)
#define SMMU_PMDIR_SIZE BIT(SMMU_PMDIR_SHIFT)
-#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE-1))
+#define SMMU_PMDIR_MASK (~(SMMU_PMDIR_SIZE - 1))
#define SMMU_PGD_TYPE (BIT(0) | BIT(1))
#define SMMU_PMD_TYPE (BIT(0) | BIT(1))
#define SMMU_PTE_TYPE (BIT(0) | BIT(1))
@@ -41,7 +43,7 @@
#define SMMU_PTE_RDONLY BIT(7) /* AP[2] */
#define SMMU_PTE_SHARED (BIT(8) | BIT(9)) /* SH[1:0], inner shareable */
#define SMMU_PTE_AF BIT(10) /* Access Flag */
-#define SMMU_PTE_NG BIT(11) /* nG */
+#define SMMU_PTE_NG BIT(11) /* nG */
#define SMMU_PTE_ATTRINDX(t) ((t) << 2)
/*
* Memory types available.
@@ -52,7 +54,6 @@
#define HISI_MT_NORMAL_NC 5
#define HISI_MT_DEVICE_nGnRE 6
-
#define SMMU_PAGE_DEFAULT (SMMU_PTE_TYPE | SMMU_PTE_AF | SMMU_PTE_SHARED)
#define SMMU_PROT_DEVICE_nGnRE (SMMU_PAGE_DEFAULT | SMMU_PTE_PXN | \
@@ -82,7 +83,7 @@ typedef u64 smmu_pte_t;
/*smmu device object*/
struct hisi_smmu_device_lpae {
- struct device *dev ;
+ struct device *dev;
struct list_head domain_list;
unsigned int ref_count;
spinlock_t lock;
@@ -102,26 +103,30 @@ struct iommu_domain_data {
};
struct hisi_map_tile_position_lpae {
- struct scatterlist *sg ;
+ struct scatterlist *sg;
unsigned long offset;
};
extern struct hisi_smmu_device_lpae *hisi_smmu_dev;
-static inline unsigned int smmu_pgd_none_lpae(smmu_pgd_t pgd) {
+static inline unsigned int smmu_pgd_none_lpae(smmu_pgd_t pgd)
+{
return !(pgd ? pgd : 0);
}
-static inline unsigned int smmu_pmd_none_lpae(smmu_pmd_t pmd) {
+static inline unsigned int smmu_pmd_none_lpae(smmu_pmd_t pmd)
+{
return !(pmd ? pmd : 0);
}
-static inline unsigned int smmu_pte_none_lpae(smmu_pte_t pte) {
+static inline unsigned int smmu_pte_none_lpae(smmu_pte_t pte)
+{
return !(pte ? pte : 0);
}
-static inline unsigned int pte_is_valid_lpae(smmu_pte_t *ptep) {
- return (unsigned int)((*(ptep)&SMMU_PTE_TYPE) ? 1 : 0);
+static inline unsigned int pte_is_valid_lpae(smmu_pte_t *ptep)
+{
+ return (unsigned int)((*(ptep) & SMMU_PTE_TYPE) ? 1 : 0);
}
/* Find an entry in the second-level page table.. */
@@ -136,7 +141,6 @@ static inline void *smmu_pte_page_vaddr_lpae(smmu_pmd_t *pmd)
return phys_to_virt(*pmd & PAGE_TABLE_ADDR_MASK);
}
-
/*fill the pgd entry, pgd value must be 64bit */
static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)
{
@@ -148,7 +152,7 @@ static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)
/*fill the pmd entry, pgd value must be 64bit */
static inline void smmu_set_pmd_lpae(smmu_pgd_t *pmdp, u64 pmd)
{
- dbg("smmu_set_pmd_lpae: pmd = 0x%lx \n", pmd);
+ dbg("smmu_set_pmd_lpae: pmd = 0x%lx\n", pmd);
*pmdp = pmd;
dsb(ishst);
isb();
@@ -179,10 +183,10 @@ static inline unsigned long smmu_pmd_addr_end_lpae(unsigned long addr, unsigned
}
int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
- unsigned long iova, phys_addr_t paddr,
- size_t size, int prot);
+ unsigned long iova, phys_addr_t paddr,
+ size_t size, int prot);
unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size);
+ unsigned long iova, size_t size);
#endif
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index fcaf97f92e7f..5fdd91a6aa8e 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -1,4 +1,4 @@
-
+// SPDX-License-Identifier: GPL-2.0
/*
* hisi_smmu_lpae.c -- 3 layer pagetable
*
@@ -30,7 +30,7 @@
struct hisi_smmu_device_lpae *hisi_smmu_dev;
-/*transfer 64bit pte table pointer to struct page*/
+/* transfer 64bit pte table pointer to struct page */
static pgtable_t smmu_pgd_to_pte_lpae(unsigned int ppte_table)
{
unsigned long page_table_addr;
@@ -43,7 +43,7 @@ static pgtable_t smmu_pgd_to_pte_lpae(unsigned int ppte_table)
return phys_to_page(page_table_addr);
}
-/*transfer 64bit pte table pointer to struct page*/
+/* transfer 64bit pte table pointer to struct page */
static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table)
{
struct page *table = NULL;
@@ -57,40 +57,42 @@ static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table)
}
static int get_domain_data_lpae(struct device_node *np,
- struct iommu_domain_data *data)
+ struct iommu_domain_data *data)
{
unsigned long long align;
struct device_node *node = NULL;
int ret = 0;
data->phy_pgd_base = hisi_smmu_dev->smmu_phy_pgtable_addr;
- if (np) {
- node = of_find_node_by_name(np, "iommu_info");
- if (!node) {
- dbg("find iommu_info node error\n");
- return -ENODEV;
- }
- ret = of_property_read_u32(node, "start-addr",
- &data->iova_start);
- if (ret) {
- dbg("read iova start address error\n");
- goto read_error;
- }
- ret = of_property_read_u32(node, "size", &data->iova_size);
- if (ret) {
- dbg("read iova size error\n");
- goto read_error;
- }
- ret = of_property_read_u64(node, "iova-align", &align);
- if (!ret)
- data->iova_align = (unsigned long)align;
- else
- data->iova_align = SZ_256K;
- pr_err("%s:start_addr 0x%x, size 0x%x align 0x%lx\n",
- __func__, data->iova_start,
- data->iova_size, data->iova_align);
+ if (!np)
+ return 0;
+
+ node = of_find_node_by_name(np, "iommu_info");
+ if (!node) {
+ dbg("find iommu_info node error\n");
+ return -ENODEV;
+ }
+ ret = of_property_read_u32(node, "start-addr",
+ &data->iova_start);
+ if (ret) {
+ dbg("read iova start address error\n");
+ goto read_error;
}
+ ret = of_property_read_u32(node, "size", &data->iova_size);
+ if (ret) {
+ dbg("read iova size error\n");
+ goto read_error;
+ }
+ ret = of_property_read_u64(node, "iova-align", &align);
+ if (!ret)
+ data->iova_align = (unsigned long)align;
+ else
+ data->iova_align = SZ_256K;
+
+ pr_err("%s:start_addr 0x%x, size 0x%x align 0x%lx\n",
+ __func__, data->iova_start,
+ data->iova_size, data->iova_align);
return 0;
@@ -99,7 +101,7 @@ static int get_domain_data_lpae(struct device_node *np,
}
static struct iommu_domain
-*hisi_smmu_domain_alloc_lpae(unsigned iommu_domain_type)
+*hisi_smmu_domain_alloc_lpae(unsigned int iommu_domain_type)
{
struct iommu_domain *domain;
@@ -107,15 +109,10 @@ static struct iommu_domain
return NULL;
domain = kzalloc(sizeof(*domain), GFP_KERNEL);
- if (!domain) {
- pr_err("%s: fail to kzalloc %lu bytes\n",
- __func__, sizeof(*domain));
- }
return domain;
}
-
static void hisi_smmu_flush_pgtable_lpae(void *addr, size_t size)
{
__flush_dcache_area(addr, size);
@@ -133,7 +130,6 @@ static void hisi_smmu_free_ptes_lpae(smmu_pgd_t pmd)
smmu_set_pmd_lpae(&pmd, 0);
}
-
static void hisi_smmu_free_pmds_lpae(smmu_pgd_t pgd)
{
pgtable_t table = smmu_pmd_to_pte_lpae(pgd);
@@ -174,15 +170,13 @@ static void hisi_smmu_free_pgtables_lpae(unsigned long *page_table_addr)
static void hisi_smmu_domain_free_lpae(struct iommu_domain *domain)
{
if (list_empty(&hisi_smmu_dev->domain_list))
- hisi_smmu_free_pgtables_lpae((unsigned long *)
- hisi_smmu_dev->va_pgtable_addr);
+ hisi_smmu_free_pgtables_lpae((unsigned long *)hisi_smmu_dev->va_pgtable_addr);
kfree(domain);
-
}
static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
- unsigned long addr, unsigned long end,
+ unsigned long addr, unsigned long end,
unsigned long pfn, u64 prot, unsigned long *flags)
{
smmu_pte_t *pte, *start;
@@ -203,11 +197,12 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
if (smmu_pmd_none_lpae(*ppmd)) {
hisi_smmu_flush_pgtable_lpae(page_address(table),
- SMMU_PAGE_SIZE);
- smmu_pmd_populate_lpae(ppmd, table, SMMU_PMD_TYPE|SMMU_PMD_NS);
+ SMMU_PAGE_SIZE);
+ smmu_pmd_populate_lpae(ppmd, table, SMMU_PMD_TYPE | SMMU_PMD_NS);
hisi_smmu_flush_pgtable_lpae(ppmd, sizeof(*ppmd));
- } else
+ } else {
__free_page(table);
+ }
pte_ready:
if (prot & IOMMU_SEC)
@@ -248,7 +243,7 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
do {
if (!pte_is_valid_lpae(pte))
- *pte = (u64)(__pfn_to_phys(pfn)|pteval);
+ *pte = (u64)(__pfn_to_phys(pfn) | pteval);
else
WARN_ONCE(1, "map to same VA more times!\n");
pte++;
@@ -261,8 +256,9 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
}
static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
- unsigned long addr, unsigned long end,
- unsigned long paddr, int prot, unsigned long *flags)
+ unsigned long addr, unsigned long end,
+ unsigned long paddr, int prot,
+ unsigned long *flags)
{
int ret = 0;
smmu_pmd_t *ppmd, *start;
@@ -283,11 +279,12 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
if (smmu_pgd_none_lpae(*ppgd)) {
hisi_smmu_flush_pgtable_lpae(page_address(table),
- SMMU_PAGE_SIZE);
- smmu_pgd_populate_lpae(ppgd, table, SMMU_PGD_TYPE|SMMU_PGD_NS);
+ SMMU_PAGE_SIZE);
+ smmu_pgd_populate_lpae(ppgd, table, SMMU_PGD_TYPE | SMMU_PGD_NS);
hisi_smmu_flush_pgtable_lpae(ppgd, sizeof(*ppgd));
- } else
+ } else {
__free_page(table);
+ }
pmd_ready:
if (prot & IOMMU_SEC)
@@ -298,8 +295,9 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
do {
next = smmu_pmd_addr_end_lpae(addr, end);
- ret = hisi_smmu_alloc_init_pte_lpae(ppmd,
- addr, next, __phys_to_pfn(paddr), prot, flags);
+ ret = hisi_smmu_alloc_init_pte_lpae(ppmd, addr, next,
+ __phys_to_pfn(paddr),
+ prot, flags);
if (ret)
goto error;
paddr += (next - addr);
@@ -310,8 +308,8 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
}
int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
- unsigned long iova, phys_addr_t paddr,
- size_t size, int prot)
+ unsigned long iova, phys_addr_t paddr,
+ size_t size, int prot)
{
int ret;
unsigned long end;
@@ -331,7 +329,7 @@ int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
do {
next = smmu_pgd_addr_end_lpae(iova, end);
ret = hisi_smmu_alloc_init_pmd_lpae(pgd,
- iova, next, paddr, prot, &flags);
+ iova, next, paddr, prot, &flags);
if (ret)
goto out_unlock;
paddr += next - iova;
@@ -359,12 +357,12 @@ static int hisi_smmu_map_lpae(struct iommu_domain *domain,
max_iova = data->iova_start + data->iova_size;
if (iova < data->iova_start) {
dbg("iova failed: iova = 0x%lx, start = 0x%8x\n",
- iova, data->iova_start);
+ iova, data->iova_start);
goto error;
}
- if ((iova+size) > max_iova) {
+ if ((iova + size) > max_iova) {
dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
- iova+size, max_iova);
+ iova + size, max_iova);
goto error;
}
return hisi_smmu_handle_mapping_lpae(domain, iova, paddr, size, prot);
@@ -374,7 +372,7 @@ static int hisi_smmu_map_lpae(struct iommu_domain *domain,
}
static unsigned int hisi_smmu_clear_pte_lpae(smmu_pgd_t *pmdp,
- unsigned int iova, unsigned int end)
+ unsigned int iova, unsigned int end)
{
smmu_pte_t *ptep = NULL;
smmu_pte_t *ppte = NULL;
@@ -390,7 +388,7 @@ static unsigned int hisi_smmu_clear_pte_lpae(smmu_pgd_t *pmdp,
}
static unsigned int hisi_smmu_clear_pmd_lpae(smmu_pgd_t *pgdp,
- unsigned int iova, unsigned int end)
+ unsigned int iova, unsigned int end)
{
smmu_pmd_t *pmdp = NULL;
smmu_pmd_t *ppmd = NULL;
@@ -410,7 +408,7 @@ static unsigned int hisi_smmu_clear_pmd_lpae(smmu_pgd_t *pgdp,
}
unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size)
+ unsigned long iova, size_t size)
{
smmu_pgd_t *pgdp = NULL;
unsigned int end = 0;
@@ -437,8 +435,8 @@ unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
}
static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size,
- struct iommu_iotlb_gather *iotlb_gather)
+ unsigned long iova, size_t size,
+ struct iommu_iotlb_gather *iotlb_gather)
{
unsigned long max_iova;
unsigned int ret;
@@ -449,14 +447,14 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
return -ENODEV;
}
data = domain->priv;
- /*caculate the max io virtual address */
+ /*calculate the max io virtual address */
max_iova = data->iova_start + data->iova_size;
/*check the iova */
if (iova < data->iova_start)
goto error;
- if ((iova+size) > max_iova) {
+ if ((iova + size) > max_iova) {
dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
- iova+size, max_iova);
+ iova + size, max_iova);
goto error;
}
/*unmapping the range of iova*/
@@ -472,8 +470,8 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
return -EINVAL;
}
-static phys_addr_t hisi_smmu_iova_to_phys_lpae(
- struct iommu_domain *domain, dma_addr_t iova)
+static phys_addr_t hisi_smmu_iova_to_phys_lpae(struct iommu_domain *domain,
+ dma_addr_t iova)
{
smmu_pgd_t *pgdp, pgd;
smmu_pmd_t pmd;
@@ -505,7 +503,7 @@ static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev)
int ret = 0;
struct iommu_domain_data *iommu_info = NULL;
- iommu_info = kzalloc(sizeof(struct iommu_domain_data), GFP_KERNEL);
+ iommu_info = kzalloc(sizeof(*iommu_info), GFP_KERNEL);
if (!iommu_info) {
dbg("alloc iommu_domain_data fail\n");
return -EINVAL;
@@ -517,7 +515,7 @@ static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev)
}
static void hisi_detach_dev_lpae(struct iommu_domain *domain,
- struct device *dev)
+ struct device *dev)
{
struct iommu_domain_data *data;
@@ -547,7 +545,8 @@ int iommu_map_tile(struct iommu_domain *domain, unsigned long iova,
if (unlikely(!(domain->ops->map_tile)))
return -ENODEV;
- BUG_ON(iova & (~PAGE_MASK));
+ if (WARN_ON(iova & (~PAGE_MASK)))
+ return -EINVAL;
return domain->ops->map_tile(domain, iova, sg, size, prot, format);
}
@@ -558,25 +557,29 @@ int iommu_unmap_tile(struct iommu_domain *domain, unsigned long iova,
if (unlikely(!(domain->ops->unmap_tile)))
return -ENODEV;
- BUG_ON(iova & (~PAGE_MASK));
+ if (WARN_ON(iova & (~PAGE_MASK)))
+ return -EINVAL;
return domain->ops->unmap_tile(domain, iova, size);
}
/*
- *iova: the start address for tile mapping
- *size: the physical memory size
- *sg: the node of scatter list where are the start node of physical memory
- *sg_offset:the physical memory offset in the sg node ,where is the start
- position of physical memory
- *port: the pape property of virtual memory
+ * iova: the start address for tile mapping
+ * size: the physical memory size
+ * sg: the node of scatter list where are the start node of physical memory
+ * sg_offset: the physical memory offset in the sg node ,where is the start
+ * position of physical memory
+ * prot: the pape property of virtual memory
+ *
* this function complete one row mapping.
*/
-static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
- iova, size_t size, struct scatterlist *sg, size_t sg_offset,
- struct hisi_map_tile_position_lpae *map_position,
- unsigned int prot){
-
+static size_t
+hisi_map_tile_row_lpae(struct iommu_domain *domain,
+ unsigned long iova, size_t size, struct scatterlist *sg,
+ size_t sg_offset,
+ struct hisi_map_tile_position_lpae *map_position,
+ unsigned int prot)
+{
unsigned long map_size; /*the memory size that will be mapped*/
unsigned long phys_addr;
unsigned long mapped_size = 0; /*memory size that has been mapped*/
@@ -591,15 +594,16 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
if (map_size > (sg->length - sg_offset))
map_size = (sg->length - sg_offset);
- /*get the start physical address*/
+ /* get the start physical address */
phys_addr = (unsigned long)get_phys_addr_lpae(sg) + sg_offset;
ret = hisi_smmu_map_lpae(domain,
- iova + mapped_size, phys_addr, map_size, prot, GFP_KERNEL);
+ iova + mapped_size, phys_addr,
+ map_size, prot, GFP_KERNEL);
if (ret) {
dbg("[%s] hisi_smmu_map failed!\n", __func__);
break;
}
- /*update mapped memory size*/
+ /* update mapped memory size */
mapped_size += map_size;
/*
* if finished mapping,
@@ -616,7 +620,7 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
}
} else {
sg_offset += map_size;
- /*if physcial memory of this node is exhausted,
+ /* if physcial memory of this node is exhausted,
* we choose next node
*/
if (sg_offset == sg->length) {
@@ -626,7 +630,7 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
break;
}
}
- /*save current position*/
+ /* save current position */
map_position->sg = sg;
map_position->offset = sg_offset;
@@ -634,19 +638,20 @@ static size_t hisi_map_tile_row_lpae(struct iommu_domain *domain, unsigned long
}
/*
- *domain:the iommu domain for mapping
- *iova:the start virtual address
- *sg: the scatter list of physical memory
- *size:the total size of all virtual memory
- *port:the property of page table of virtual memory
- *format:the parameter of tile mapping
- *this function map physical memory in tile mode
+ * domain:the iommu domain for mapping
+ * iova:the start virtual address
+ * sg: the scatter list of physical memory
+ * size:the total size of all virtual memory
+ * port:the property of page table of virtual memory
+ * format:the parameter of tile mapping
+ * this function map physical memory in tile mode
*/
static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
- unsigned long iova,
- struct scatterlist *sg, size_t size, int prot,
- struct tile_format *format){
-
+ unsigned long iova,
+ struct scatterlist *sg,
+ size_t size, int prot,
+ struct tile_format *format)
+{
unsigned int phys_length;
struct scatterlist *sg_node;
unsigned int row_number, row;
@@ -662,29 +667,29 @@ static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
header_size = format->header_size;
- /* calculate the number of raws*/
+ /* calculate the number of raws */
row_number = ((phys_length - header_size) >> PAGE_SHIFT)
/ format->phys_page_line;
dbg("phys_length: 0x%x, rows: 0x%x, header_size: 0x%x\n",
- phys_length, row_number, header_size);
+ phys_length, row_number, header_size);
- /*caculate the need physical memory and virtual memory for one row*/
+ /* calculate the need physical memory and virtual memory for one row */
size_phys = (format->phys_page_line * PAGE_SIZE);
size_virt = (format->virt_page_line * PAGE_SIZE);
sg_offset = 0;
sg_node = sg;
- /*set start position*/
+ /* set start position */
map_position.sg = sg;
map_position.offset = 0;
- /*map header*/
+ /* map header */
if (header_size) {
mapped_size = hisi_map_tile_row_lpae(domain, iova,
- header_size, sg_node,
- sg_offset, &map_position,
- prot);
+ header_size, sg_node,
+ sg_offset, &map_position,
+ prot);
if (mapped_size != header_size) {
WARN(1, "map head fail\n");
ret = -EINVAL;
@@ -704,9 +709,9 @@ static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
}
/* map one row*/
mapped_size = hisi_map_tile_row_lpae(domain,
- iova + (size_virt * row),
- size_phys, sg_node, sg_offset,
- &map_position, prot);
+ iova + (size_virt * row),
+ size_phys, sg_node, sg_offset,
+ &map_position, prot);
if (mapped_size != size_phys) {
WARN(1, "hisi_map_tile_row failed!\n");
ret = -EINVAL;
@@ -718,7 +723,7 @@ static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
}
static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size)
+ unsigned long iova, size_t size)
{
return hisi_smmu_unmap_lpae(domain, iova, size, NULL);
}
@@ -781,14 +786,13 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
dbg("enter %s\n", __func__);
hisi_smmu_dev = devm_kzalloc(dev,
- sizeof(struct hisi_smmu_device_lpae), GFP_KERNEL);
+ sizeof(struct hisi_smmu_device_lpae),
+ GFP_KERNEL);
hisi_smmu_dev->smmu_pgd = devm_kzalloc(dev, SZ_64,
GFP_KERNEL | __GFP_DMA);
- if (!hisi_smmu_dev) {
- ret = -ENOMEM;
- goto smmu_device_error;
- }
+ if (!hisi_smmu_dev)
+ return -ENOMEM;
hisi_smmu_dev->dev = dev;
INIT_LIST_HEAD(&hisi_smmu_dev->domain_list);
@@ -804,7 +808,7 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
hisi_smmu_dev->va_pgtable_addr = (unsigned long)(hisi_smmu_dev->smmu_pgd);
ret = iommu_device_sysfs_add(&hisi_smmu_dev->iommu, NULL, NULL,
- "hisi-iommu");
+ "hisi-iommu");
if (ret)
goto fail_register;
@@ -821,8 +825,6 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
fail_register:
iommu_device_sysfs_remove(&hisi_smmu_dev->iommu);
-
-smmu_device_error:
return ret;
}
@@ -851,10 +853,7 @@ static struct platform_driver hisi_smmu_driver_lpae = {
static int __init hisi_smmu_init_lpae(void)
{
- int ret = 0;
-
- ret = platform_driver_register(&hisi_smmu_driver_lpae);
- return ret;
+ return platform_driver_register(&hisi_smmu_driver_lpae);
}
static void __exit hisi_smmu_exit_lpae(void)
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 07/16] iommu: hisi_smmu_lpae: get rid of IOMMU_SEC and IOMMU_DEVICE
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (7 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Those prot bits aren't needed for Hikey970's GPU code, and depends
on some patch not on upstream.
So, get rid of them.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 41 +++++++++--------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 5fdd91a6aa8e..9dae0a3067b6 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -205,9 +205,6 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
}
pte_ready:
- if (prot & IOMMU_SEC)
- *ppmd &= (~SMMU_PMD_NS);
-
start = (smmu_pte_t *)smmu_pte_page_vaddr_lpae(ppmd)
+ smmu_pte_index(addr);
pte = start;
@@ -215,30 +212,24 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
pteval |= SMMU_PROT_NORMAL;
pteval |= SMMU_PTE_NS;
} else {
- if (prot & IOMMU_DEVICE) {
- pteval |= SMMU_PROT_DEVICE_nGnRE;
- } else {
- if (prot & IOMMU_CACHE)
- pteval |= SMMU_PROT_NORMAL_CACHE;
- else
- pteval |= SMMU_PROT_NORMAL_NC;
+ if (prot & IOMMU_CACHE)
+ pteval |= SMMU_PROT_NORMAL_CACHE;
+ else
+ pteval |= SMMU_PROT_NORMAL_NC;
- if ((prot & IOMMU_READ) && (prot & IOMMU_WRITE))
- pteval |= SMMU_PAGE_READWRITE;
- else if ((prot & IOMMU_READ) && !(prot & IOMMU_WRITE))
- pteval |= SMMU_PAGE_READONLY;
- else
- WARN_ON("you do not set read attribute!");
+ if ((prot & IOMMU_READ) && (prot & IOMMU_WRITE))
+ pteval |= SMMU_PAGE_READWRITE;
+ else if ((prot & IOMMU_READ) && !(prot & IOMMU_WRITE))
+ pteval |= SMMU_PAGE_READONLY;
+ else
+ WARN_ON("you do not set read attribute!");
- if (prot & IOMMU_EXEC) {
- pteval |= SMMU_PAGE_READONLY_EXEC;
- pteval &= ~(SMMU_PTE_PXN | SMMU_PTE_UXN);
- }
+ if (!(prot & IOMMU_NOEXEC)) {
+ pteval |= SMMU_PAGE_READONLY_EXEC;
+ pteval &= ~(SMMU_PTE_PXN | SMMU_PTE_UXN);
}
- if (prot & IOMMU_SEC)
- pteval &= (~SMMU_PTE_NS);
- else
- pteval |= SMMU_PTE_NS;
+
+ pteval |= SMMU_PTE_NS;
}
do {
@@ -287,8 +278,6 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
}
pmd_ready:
- if (prot & IOMMU_SEC)
- *ppgd &= (~SMMU_PGD_NS);
start = (smmu_pmd_t *)smmu_pmd_page_vaddr_lpae(ppgd)
+ smmu_pmd_index(addr);
ppmd = start;
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 08/16] iommu: get rid of map/unmap tile functions
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (8 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Those are needed by the ION-specific downstream code.
Such code would require changes at the core iommu header
file. As we won't be using the ION-specific binding, we can
just get rid of those.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 201 ----------------------
1 file changed, 201 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 9dae0a3067b6..a55b5a35b339 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -518,205 +518,6 @@ static void hisi_detach_dev_lpae(struct iommu_domain *domain,
}
}
-static dma_addr_t get_phys_addr_lpae(struct scatterlist *sg)
-{
- dma_addr_t dma_addr = sg_dma_address(sg);
-
- if (!dma_addr)
- dma_addr = sg_phys(sg);
- return dma_addr;
-}
-
-int iommu_map_tile(struct iommu_domain *domain, unsigned long iova,
- struct scatterlist *sg, size_t size, int prot,
- struct tile_format *format)
-{
- if (unlikely(!(domain->ops->map_tile)))
- return -ENODEV;
-
- if (WARN_ON(iova & (~PAGE_MASK)))
- return -EINVAL;
-
- return domain->ops->map_tile(domain, iova, sg, size, prot, format);
-}
-
-int iommu_unmap_tile(struct iommu_domain *domain, unsigned long iova,
- size_t size)
-{
- if (unlikely(!(domain->ops->unmap_tile)))
- return -ENODEV;
-
- if (WARN_ON(iova & (~PAGE_MASK)))
- return -EINVAL;
-
- return domain->ops->unmap_tile(domain, iova, size);
-}
-
-/*
- * iova: the start address for tile mapping
- * size: the physical memory size
- * sg: the node of scatter list where are the start node of physical memory
- * sg_offset: the physical memory offset in the sg node ,where is the start
- * position of physical memory
- * prot: the pape property of virtual memory
- *
- * this function complete one row mapping.
- */
-static size_t
-hisi_map_tile_row_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size, struct scatterlist *sg,
- size_t sg_offset,
- struct hisi_map_tile_position_lpae *map_position,
- unsigned int prot)
-{
- unsigned long map_size; /*the memory size that will be mapped*/
- unsigned long phys_addr;
- unsigned long mapped_size = 0; /*memory size that has been mapped*/
- int ret;
-
- while (1) {
- /*
- *get the remain memory,if current sg node is not enough memory,
- *we map the remain memory firstly.
- */
- map_size = size - mapped_size;
- if (map_size > (sg->length - sg_offset))
- map_size = (sg->length - sg_offset);
-
- /* get the start physical address */
- phys_addr = (unsigned long)get_phys_addr_lpae(sg) + sg_offset;
- ret = hisi_smmu_map_lpae(domain,
- iova + mapped_size, phys_addr,
- map_size, prot, GFP_KERNEL);
- if (ret) {
- dbg("[%s] hisi_smmu_map failed!\n", __func__);
- break;
- }
- /* update mapped memory size */
- mapped_size += map_size;
- /*
- * if finished mapping,
- * we update the memory offset of current node and
- * save the memory position. otherwise we clean the sg_offset
- * to zero and get next sg node.
- */
- if (mapped_size < size) {
- sg_offset = 0;
- sg = sg_next(sg);
- if (!sg) {
- dbg("[%s] phy memory not enough\n", __func__);
- break;
- }
- } else {
- sg_offset += map_size;
- /* if physcial memory of this node is exhausted,
- * we choose next node
- */
- if (sg_offset == sg->length) {
- sg_offset = 0;
- sg = sg_next(sg);
- }
- break;
- }
- }
- /* save current position */
- map_position->sg = sg;
- map_position->offset = sg_offset;
-
- return mapped_size;
-}
-
-/*
- * domain:the iommu domain for mapping
- * iova:the start virtual address
- * sg: the scatter list of physical memory
- * size:the total size of all virtual memory
- * port:the property of page table of virtual memory
- * format:the parameter of tile mapping
- * this function map physical memory in tile mode
- */
-static int hisi_smmu_map_tile_lpae(struct iommu_domain *domain,
- unsigned long iova,
- struct scatterlist *sg,
- size_t size, int prot,
- struct tile_format *format)
-{
- unsigned int phys_length;
- struct scatterlist *sg_node;
- unsigned int row_number, row;
- unsigned int size_virt, size_phys;
- unsigned int sg_offset;
- int ret = size;
- unsigned int mapped_size, header_size;
- struct hisi_map_tile_position_lpae map_position;
-
- /* calculate the whole length of phys mem */
- for (phys_length = 0, sg_node = sg; sg_node; sg_node = sg_next(sg_node))
- phys_length += ALIGN(sg_node->length, PAGE_SIZE);
-
- header_size = format->header_size;
-
- /* calculate the number of raws */
- row_number = ((phys_length - header_size) >> PAGE_SHIFT)
- / format->phys_page_line;
- dbg("phys_length: 0x%x, rows: 0x%x, header_size: 0x%x\n",
- phys_length, row_number, header_size);
-
- /* calculate the need physical memory and virtual memory for one row */
- size_phys = (format->phys_page_line * PAGE_SIZE);
- size_virt = (format->virt_page_line * PAGE_SIZE);
-
- sg_offset = 0;
- sg_node = sg;
-
- /* set start position */
- map_position.sg = sg;
- map_position.offset = 0;
-
- /* map header */
- if (header_size) {
- mapped_size = hisi_map_tile_row_lpae(domain, iova,
- header_size, sg_node,
- sg_offset, &map_position,
- prot);
- if (mapped_size != header_size) {
- WARN(1, "map head fail\n");
- ret = -EINVAL;
- goto error;
- }
- iova += ALIGN(header_size, size_virt);
- }
- /* map row by row */
- for (row = 0; row < row_number; row++) {
- /* get physical memory position */
- if (map_position.sg) {
- sg_node = map_position.sg;
- sg_offset = map_position.offset;
- } else {
- dbg("[%s]:physical memory is not enough\n", __func__);
- break;
- }
- /* map one row*/
- mapped_size = hisi_map_tile_row_lpae(domain,
- iova + (size_virt * row),
- size_phys, sg_node, sg_offset,
- &map_position, prot);
- if (mapped_size != size_phys) {
- WARN(1, "hisi_map_tile_row failed!\n");
- ret = -EINVAL;
- break;
- }
- };
-error:
- return ret;
-}
-
-static size_t hisi_smmu_unmap_tile_lpae(struct iommu_domain *domain,
- unsigned long iova, size_t size)
-{
- return hisi_smmu_unmap_lpae(domain, iova, size, NULL);
-}
-
static bool hisi_smmu_capable(enum iommu_cap cap)
{
return false;
@@ -764,8 +565,6 @@ static struct iommu_ops hisi_smmu_ops = {
.remove_device = hisi_smmu_remove_device,
.device_group = generic_device_group,
.pgsize_bitmap = SMMU_PAGE_SIZE,
- .map_tile = hisi_smmu_map_tile_lpae,
- .unmap_tile = hisi_smmu_unmap_tile_lpae,
};
static int hisi_smmu_probe_lpae(struct platform_device *pdev)
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 09/16] iommu: hisi_smmu_lpae: use the right code to get domain-priv data
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (9 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
The downstream code needed to use a priv data within the
domain struct. Change it to work like other iommu drivers:
use dev_iommu_priv_get() and dev_iommu_priv_set() instead.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 17 ++++++++--
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 38 +++++++++++++----------
2 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
index b2d32ec6cb84..290f2e11c3be 100644
--- a/drivers/staging/hikey9xx/hisi_smmu.h
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -94,7 +94,7 @@ struct hisi_smmu_device_lpae {
struct iommu_device iommu;
};
-struct iommu_domain_data {
+struct hisi_smmu_domain_data {
unsigned int iova_start;
unsigned int iova_size;
phys_addr_t phy_pgd_base;
@@ -102,13 +102,24 @@ struct iommu_domain_data {
struct list_head list;
};
+struct hisi_smmu_domain {
+ struct iommu_domain domain;
+ struct hisi_smmu_domain_data *iommu_info;
+};
+
+static struct hisi_smmu_domain_data *to_smmu(struct iommu_domain *dom)
+{
+ struct hisi_smmu_domain *hisi_dom;
+
+ hisi_dom = container_of(dom, struct hisi_smmu_domain, domain);
+ return hisi_dom->iommu_info;
+}
+
struct hisi_map_tile_position_lpae {
struct scatterlist *sg;
unsigned long offset;
};
-extern struct hisi_smmu_device_lpae *hisi_smmu_dev;
-
static inline unsigned int smmu_pgd_none_lpae(smmu_pgd_t pgd)
{
return !(pgd ? pgd : 0);
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index a55b5a35b339..1fe57c10e745 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -28,7 +28,7 @@
#include <linux/bitops.h>
#include "hisi_smmu.h"
-struct hisi_smmu_device_lpae *hisi_smmu_dev;
+static struct hisi_smmu_device_lpae *hisi_smmu_dev;
/* transfer 64bit pte table pointer to struct page */
static pgtable_t smmu_pgd_to_pte_lpae(unsigned int ppte_table)
@@ -57,7 +57,7 @@ static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table)
}
static int get_domain_data_lpae(struct device_node *np,
- struct iommu_domain_data *data)
+ struct hisi_smmu_domain_data *data)
{
unsigned long long align;
struct device_node *node = NULL;
@@ -103,14 +103,16 @@ static int get_domain_data_lpae(struct device_node *np,
static struct iommu_domain
*hisi_smmu_domain_alloc_lpae(unsigned int iommu_domain_type)
{
- struct iommu_domain *domain;
+ struct hisi_smmu_domain *hisi_dom;
if (iommu_domain_type != IOMMU_DOMAIN_UNMANAGED)
return NULL;
- domain = kzalloc(sizeof(*domain), GFP_KERNEL);
+ hisi_dom = kzalloc(sizeof(*hisi_dom), GFP_KERNEL);
- return domain;
+ pr_debug("%s: domain allocated\n", __func__);
+
+ return &hisi_dom->domain;
}
static void hisi_smmu_flush_pgtable_lpae(void *addr, size_t size)
@@ -336,13 +338,13 @@ static int hisi_smmu_map_lpae(struct iommu_domain *domain,
gfp_t gfp)
{
unsigned long max_iova;
- struct iommu_domain_data *data;
+ struct hisi_smmu_domain_data *data;
if (!domain) {
dbg("domain is null\n");
return -ENODEV;
}
- data = domain->priv;
+ data = to_smmu(domain);
max_iova = data->iova_start + data->iova_size;
if (iova < data->iova_start) {
dbg("iova failed: iova = 0x%lx, start = 0x%8x\n",
@@ -429,13 +431,13 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
{
unsigned long max_iova;
unsigned int ret;
- struct iommu_domain_data *data;
+ struct hisi_smmu_domain_data *data;
if (!domain) {
dbg("domain is null\n");
return -ENODEV;
}
- data = domain->priv;
+ data = to_smmu(domain);
/*calculate the max io virtual address */
max_iova = data->iova_start + data->iova_size;
/*check the iova */
@@ -490,28 +492,32 @@ static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev)
{
struct device_node *np = dev->of_node;
int ret = 0;
- struct iommu_domain_data *iommu_info = NULL;
+ struct hisi_smmu_domain_data *iommu_info = NULL;
+ struct hisi_smmu_domain *hisi_dom;
iommu_info = kzalloc(sizeof(*iommu_info), GFP_KERNEL);
if (!iommu_info) {
- dbg("alloc iommu_domain_data fail\n");
+ dbg("alloc hisi_smmu_domain_data fail\n");
return -EINVAL;
}
list_add(&iommu_info->list, &hisi_smmu_dev->domain_list);
- domain->priv = iommu_info;
- ret = get_domain_data_lpae(np, domain->priv);
+
+ hisi_dom = container_of(domain, struct hisi_smmu_domain, domain);
+ hisi_dom->iommu_info = iommu_info;
+ dev_iommu_priv_set(dev, iommu_info);
+ ret = get_domain_data_lpae(np, iommu_info);
return ret;
}
static void hisi_detach_dev_lpae(struct iommu_domain *domain,
struct device *dev)
{
- struct iommu_domain_data *data;
+ struct hisi_smmu_domain_data *data;
- data = (struct iommu_domain_data *)domain->priv;
+ data = dev_iommu_priv_get(dev);
if (data) {
list_del(&data->list);
- domain->priv = NULL;
+ dev_iommu_priv_set(dev, NULL);
kfree(data);
} else {
dbg("%s:error! data entry has been delected\n", __func__);
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 10/16] iommu: hisi_smmu_lpae: convert it to probe_device
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (10 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
The add_device()/remove_device() was removed on Kernel 5.8.
Convert the driver to use probe_device/release_device.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 31 +++++------------------
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 1fe57c10e745..c8c7e8e3dde2 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -529,33 +529,14 @@ static bool hisi_smmu_capable(enum iommu_cap cap)
return false;
}
-static int hisi_smmu_add_device(struct device *dev)
+static struct iommu_device *hisi_smmu_probe_device(struct device *dev)
{
- struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev;
- struct iommu_group *group;
-
- if (iommu)
- iommu_device_link(&iommu->iommu, dev);
- else
- return -ENODEV;
-
- group = iommu_group_get_for_dev(dev);
- if (IS_ERR(group))
- return PTR_ERR(group);
-
- iommu_group_put(group);
-
- return 0;
+ return &hisi_smmu_dev->iommu;
}
-static void hisi_smmu_remove_device(struct device *dev)
+static void hisi_smmu_release_device(struct device *dev)
{
- struct hisi_smmu_device_lpae *iommu = hisi_smmu_dev;
-
- if (iommu)
- iommu_device_unlink(&iommu->iommu, dev);
-
- iommu_group_remove_device(dev);
+ /* As just one SMMU is possible, nothing to do here */
}
static struct iommu_ops hisi_smmu_ops = {
@@ -567,8 +548,8 @@ static struct iommu_ops hisi_smmu_ops = {
.map = hisi_smmu_map_lpae,
.unmap = hisi_smmu_unmap_lpae,
.iova_to_phys = hisi_smmu_iova_to_phys_lpae,
- .add_device = hisi_smmu_add_device,
- .remove_device = hisi_smmu_remove_device,
+ .probe_device = hisi_smmu_probe_device,
+ .release_device = hisi_smmu_release_device,
.device_group = generic_device_group,
.pgsize_bitmap = SMMU_PAGE_SIZE,
};
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 11/16] iommu: add Hisilicon Kirin970 iommu at the building system
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (11 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Now that the iommu driver is ready, add it to the building
system.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/Kconfig | 9 +++++++++
drivers/staging/hikey9xx/Makefile | 1 +
2 files changed, 10 insertions(+)
diff --git a/drivers/staging/hikey9xx/Kconfig b/drivers/staging/hikey9xx/Kconfig
index 76267b9be562..6487278a4144 100644
--- a/drivers/staging/hikey9xx/Kconfig
+++ b/drivers/staging/hikey9xx/Kconfig
@@ -33,3 +33,12 @@ config REGULATOR_HI6421V600
This driver provides support for the voltage regulators on
HiSilicon Hi6421v600 PMU / Codec IC.
This is used on Kirin 3670 boards, like HiKey 970.
+
+# to be placed at drivers/iommu
+config HISI_IOMMU_LPAE
+ bool "Hisilicon IOMMU LPAE Support"
+ select IOMMU_API
+ select IODOMAIN_API
+ help
+ This driver provides support for the IOMMU found on Kirin 970.
+ This is used on Kirin 3670 boards, like HiKey 970.
diff --git a/drivers/staging/hikey9xx/Makefile b/drivers/staging/hikey9xx/Makefile
index 9371dcc3d35b..c6e4998c02dd 100644
--- a/drivers/staging/hikey9xx/Makefile
+++ b/drivers/staging/hikey9xx/Makefile
@@ -3,3 +3,4 @@
obj-$(CONFIG_SPMI_HISI3670) += hisi-spmi-controller.o
obj-$(CONFIG_MFD_HI6421_SPMI) += hi6421-spmi-pmic.o
obj-$(CONFIG_REGULATOR_HI6421V600) += hi6421v600-regulator.o
+obj-$(CONFIG_HISI_IOMMU_LPAE) += hisi_smmu_lpae.o
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 12/16] iommu: hisi_smmu_lpae: cleanup printk macros
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (12 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
- Whenever possible, use dev_foo() instead of pr_foo();
- Replace dbg() macro by either pr_debug() or dev_dbg();
- Fix some warnings due to different integer types.
- add debug at hisi_smmu_domain_alloc_lpae() to allow checking
what domains were mapped.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu.h | 9 +--
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 72 ++++++++++++-----------
2 files changed, 39 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu.h b/drivers/staging/hikey9xx/hisi_smmu.h
index 290f2e11c3be..8724806bc507 100644
--- a/drivers/staging/hikey9xx/hisi_smmu.h
+++ b/drivers/staging/hikey9xx/hisi_smmu.h
@@ -3,13 +3,6 @@
#ifndef HISI_SMMU_H
#define HISI_SMMU_H
-/*#define IOMMU_DEBUG*/
-#ifdef IOMMU_DEBUG
-#define dbg(format, arg...) printk(KERN_ERR "[iommu]" format, ##arg)
-#else
-#define dbg(format, arg...)
-#endif
-
#define SMMU_PHY_PTRS_PER_PTE (256)
/*#define SMMU_PHY_PTRS_PER_PGD (4096)*/
#define SMMU_PTRS_PER_PGD (4)
@@ -163,7 +156,7 @@ static inline void smmu_set_pgd_lpae(smmu_pgd_t *pgdp, u64 pgd)
/*fill the pmd entry, pgd value must be 64bit */
static inline void smmu_set_pmd_lpae(smmu_pgd_t *pmdp, u64 pmd)
{
- dbg("smmu_set_pmd_lpae: pmd = 0x%lx\n", pmd);
+ pr_debug("smmu_set_pmd_lpae: pmd = 0x%llx\n", pmd);
*pmdp = pmd;
dsb(ishst);
isb();
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index c8c7e8e3dde2..33aba006d6a1 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -36,7 +36,7 @@ static pgtable_t smmu_pgd_to_pte_lpae(unsigned int ppte_table)
unsigned long page_table_addr;
if (!ppte_table) {
- dbg("error: the pointer of pte_table is NULL\n");
+ pr_debug("error: the pointer of pte_table is NULL\n");
return NULL;
}
page_table_addr = (unsigned long)ppte_table;
@@ -49,14 +49,15 @@ static pgtable_t smmu_pmd_to_pte_lpae(unsigned long ppte_table)
struct page *table = NULL;
if (!ppte_table) {
- dbg("error: the pointer of pte_table is NULL\n");
+ pr_debug("error: the pointer of pte_table is NULL\n");
return NULL;
}
table = phys_to_page(ppte_table);
return table;
}
-static int get_domain_data_lpae(struct device_node *np,
+static int get_domain_data_lpae(struct device *dev,
+ struct device_node *np,
struct hisi_smmu_domain_data *data)
{
unsigned long long align;
@@ -70,18 +71,18 @@ static int get_domain_data_lpae(struct device_node *np,
node = of_find_node_by_name(np, "iommu_info");
if (!node) {
- dbg("find iommu_info node error\n");
+ dev_dbg(dev, "find iommu_info node error\n");
return -ENODEV;
}
ret = of_property_read_u32(node, "start-addr",
&data->iova_start);
if (ret) {
- dbg("read iova start address error\n");
+ dev_dbg(dev, "read iova start address error\n");
goto read_error;
}
ret = of_property_read_u32(node, "size", &data->iova_size);
if (ret) {
- dbg("read iova size error\n");
+ dev_dbg(dev, "read iova size error\n");
goto read_error;
}
ret = of_property_read_u64(node, "iova-align", &align);
@@ -90,7 +91,7 @@ static int get_domain_data_lpae(struct device_node *np,
else
data->iova_align = SZ_256K;
- pr_err("%s:start_addr 0x%x, size 0x%x align 0x%lx\n",
+ pr_err("%s: start_addr 0x%x, size 0x%x align 0x%lx\n",
__func__, data->iova_start,
data->iova_size, data->iova_align);
@@ -105,8 +106,11 @@ static struct iommu_domain
{
struct hisi_smmu_domain *hisi_dom;
- if (iommu_domain_type != IOMMU_DOMAIN_UNMANAGED)
+ if (iommu_domain_type != IOMMU_DOMAIN_UNMANAGED) {
+ pr_debug("%s: expecting an IOMMU_DOMAIN_UNMANAGED domain\n",
+ __func__);
return NULL;
+ }
hisi_dom = kzalloc(sizeof(*hisi_dom), GFP_KERNEL);
@@ -125,7 +129,7 @@ static void hisi_smmu_free_ptes_lpae(smmu_pgd_t pmd)
pgtable_t table = smmu_pgd_to_pte_lpae(pmd);
if (!table) {
- dbg("pte table is null\n");
+ pr_debug("pte table is null\n");
return;
}
__free_page(table);
@@ -137,7 +141,7 @@ static void hisi_smmu_free_pmds_lpae(smmu_pgd_t pgd)
pgtable_t table = smmu_pmd_to_pte_lpae(pgd);
if (!table) {
- dbg("pte table is null\n");
+ pr_debug("pte table is null\n");
return;
}
__free_page(table);
@@ -193,7 +197,7 @@ static int hisi_smmu_alloc_init_pte_lpae(smmu_pmd_t *ppmd,
table = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_DMA);
spin_lock_irqsave(&hisi_smmu_dev->lock, *flags);
if (!table) {
- dbg("%s: alloc page fail\n", __func__);
+ pr_debug("%s: alloc page fail\n", __func__);
return -ENOMEM;
}
@@ -266,7 +270,7 @@ static int hisi_smmu_alloc_init_pmd_lpae(smmu_pgd_t *ppgd,
table = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_DMA);
spin_lock_irqsave(&hisi_smmu_dev->lock, *flags);
if (!table) {
- dbg("%s: alloc page fail\n", __func__);
+ pr_debug("%s: alloc page fail\n", __func__);
return -ENOMEM;
}
@@ -309,7 +313,7 @@ int hisi_smmu_handle_mapping_lpae(struct iommu_domain *domain,
smmu_pgd_t *pgd = (smmu_pgd_t *)hisi_smmu_dev->va_pgtable_addr;
if (!pgd) {
- dbg("pgd is null\n");
+ pr_debug("pgd is null\n");
return -EINVAL;
}
iova = ALIGN(iova, SMMU_PAGE_SIZE);
@@ -341,24 +345,24 @@ static int hisi_smmu_map_lpae(struct iommu_domain *domain,
struct hisi_smmu_domain_data *data;
if (!domain) {
- dbg("domain is null\n");
+ pr_debug("domain is null\n");
return -ENODEV;
}
data = to_smmu(domain);
max_iova = data->iova_start + data->iova_size;
if (iova < data->iova_start) {
- dbg("iova failed: iova = 0x%lx, start = 0x%8x\n",
- iova, data->iova_start);
+ pr_debug("iova failed: iova = 0x%lx, start = 0x%8x\n",
+ iova, data->iova_start);
goto error;
}
if ((iova + size) > max_iova) {
- dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
- iova + size, max_iova);
+ pr_debug("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
+ iova + size, max_iova);
goto error;
}
return hisi_smmu_handle_mapping_lpae(domain, iova, paddr, size, prot);
error:
- dbg("iova is not in this range\n");
+ pr_debug("iova is not in this range\n");
return -EINVAL;
}
@@ -392,7 +396,7 @@ static unsigned int hisi_smmu_clear_pmd_lpae(smmu_pgd_t *pgdp,
next = smmu_pmd_addr_end_lpae(iova, end);
hisi_smmu_clear_pte_lpae(ppmd, iova, next);
iova = next;
- dbg("%s: iova=0x%lx, end=0x%lx\n", __func__, iova, end);
+ pr_debug("%s: iova=0x%x, end=0x%x\n", __func__, iova, end);
} while (ppmd++, iova < end);
return size;
@@ -411,14 +415,14 @@ unsigned int hisi_smmu_handle_unmapping_lpae(struct iommu_domain *domain,
size = SMMU_PAGE_ALIGN(size);
pgdp = (smmu_pgd_t *)hisi_smmu_dev->va_pgtable_addr;
end = iova + size;
- dbg("%s:end=0x%x\n", __func__, end);
+ pr_debug("%s: end=0x%x\n", __func__, end);
pgdp += smmu_pgd_index(iova);
spin_lock_irqsave(&hisi_smmu_dev->lock, flags);
do {
next = smmu_pgd_addr_end_lpae(iova, end);
unmap_size += hisi_smmu_clear_pmd_lpae(pgdp, iova, next);
iova = next;
- dbg("%s: pgdp=%p, iova=0x%lx\n", __func__, pgdp, iova);
+ pr_debug("%s: pgdp=%p, iova=0x%lx\n", __func__, pgdp, iova);
} while (pgdp++, iova < end);
spin_unlock_irqrestore(&hisi_smmu_dev->lock, flags);
@@ -434,7 +438,7 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
struct hisi_smmu_domain_data *data;
if (!domain) {
- dbg("domain is null\n");
+ pr_debug("domain is null\n");
return -ENODEV;
}
data = to_smmu(domain);
@@ -444,20 +448,20 @@ static size_t hisi_smmu_unmap_lpae(struct iommu_domain *domain,
if (iova < data->iova_start)
goto error;
if ((iova + size) > max_iova) {
- dbg("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
- iova + size, max_iova);
+ pr_debug("iova out of domain range, iova+size=0x%lx, end=0x%lx\n",
+ iova + size, max_iova);
goto error;
}
/*unmapping the range of iova*/
ret = hisi_smmu_handle_unmapping_lpae(domain, iova, size);
if (ret == size) {
- dbg("%s:unmap size:0x%x\n", __func__, (unsigned int)size);
+ pr_debug("%s: unmap size:0x%x\n", __func__, (unsigned int)size);
return size;
} else {
return 0;
}
error:
- dbg("%s:the range of io address is wrong\n", __func__);
+ pr_debug("%s: the range of io address is wrong\n", __func__);
return -EINVAL;
}
@@ -496,16 +500,15 @@ static int hisi_attach_dev_lpae(struct iommu_domain *domain, struct device *dev)
struct hisi_smmu_domain *hisi_dom;
iommu_info = kzalloc(sizeof(*iommu_info), GFP_KERNEL);
- if (!iommu_info) {
- dbg("alloc hisi_smmu_domain_data fail\n");
+ if (!iommu_info)
return -EINVAL;
- }
+
list_add(&iommu_info->list, &hisi_smmu_dev->domain_list);
hisi_dom = container_of(domain, struct hisi_smmu_domain, domain);
hisi_dom->iommu_info = iommu_info;
dev_iommu_priv_set(dev, iommu_info);
- ret = get_domain_data_lpae(np, iommu_info);
+ ret = get_domain_data_lpae(dev, np, iommu_info);
return ret;
}
@@ -520,7 +523,7 @@ static void hisi_detach_dev_lpae(struct iommu_domain *domain,
dev_iommu_priv_set(dev, NULL);
kfree(data);
} else {
- dbg("%s:error! data entry has been delected\n", __func__);
+ dev_dbg(dev, "error! domain priv struct is NULL\n");
}
}
@@ -559,7 +562,7 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
struct device *dev = &pdev->dev;
int ret;
- dbg("enter %s\n", __func__);
+ dev_dbg(dev, "%s:\n", __func__);
hisi_smmu_dev = devm_kzalloc(dev,
sizeof(struct hisi_smmu_device_lpae),
GFP_KERNEL);
@@ -573,10 +576,11 @@ static int hisi_smmu_probe_lpae(struct platform_device *pdev)
INIT_LIST_HEAD(&hisi_smmu_dev->domain_list);
spin_lock_init(&hisi_smmu_dev->lock);
- hisi_smmu_dev->smmu_pgd = (smmu_pgd_t *)(ALIGN((unsigned long)(hisi_smmu_dev->smmu_pgd), SZ_32));
+ hisi_smmu_dev->smmu_pgd = (smmu_pgd_t *)(ALIGN((unsigned long)(hisi_smmu_dev->smmu_pgd), SZ_32));
hisi_smmu_dev->smmu_phy_pgtable_addr =
virt_to_phys(hisi_smmu_dev->smmu_pgd);
+
dev_info(&pdev->dev, "%s, smmu_phy_pgtable_addr is = 0x%llx\n",
__func__, hisi_smmu_dev->smmu_phy_pgtable_addr);
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 13/16] iommu: hisi_smmu_lpae: make OF compatible more standard
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (13 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
Use "manufacturer,model" pattern for the expected compatible
string for this board.
Most of compatible lines for Huawei's Hisilicon SoCs start
with "hisilicon,". Use the same pattern here for this new
driver.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/hisi_smmu_lpae.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/hikey9xx/hisi_smmu_lpae.c b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
index 33aba006d6a1..0f4d7b54193e 100644
--- a/drivers/staging/hikey9xx/hisi_smmu_lpae.c
+++ b/drivers/staging/hikey9xx/hisi_smmu_lpae.c
@@ -616,7 +616,7 @@ static int hisi_smmu_remove_lpae(struct platform_device *pdev)
}
static const struct of_device_id hisi_smmu_of_match_lpae[] = {
- { .compatible = "hisi,hisi-smmu-lpae"},
+ { .compatible = "hisilicon,smmu-lpae"},
{ },
};
MODULE_DEVICE_TABLE(of, hisi_smmu_of_match_lpae);
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 14/16] dt: add an spec for the Kirin36x0 SMMU
2020-08-17 7:49 ` Mauro Carvalho Chehab
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Manivannan Sadhasivam, Mauro Carvalho Chehab,
linuxarm, linux-kernel, iommu, Rob Herring, John Stultz,
mauro.chehab
Describe the properties expected by the IOMMU driver used on
Hikey960 and Hikey970 boards.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
diff --git a/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml b/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
new file mode 100644
index 000000000000..ec4c98faf3a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/hisilicon,kirin36x0-smmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon support for HI3660/HI3670 SMMU
+
+maintainers:
+ - Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+
+description: |+
+ Huawei's Hisilicon Kirin 3660/3670 contains a System MMU that enables
+ scattered physical memory chunks visible as a contiguous region to
+ DMA-capable peripheral devices like GPU and ISP.
+
+ The IOMMU domains are described via iommu_info settings.
+
+properties:
+ compatible:
+ const: hisilicon,hisi-smmu-lpae
+
+ iommu_info:
+ type: object
+
+ properties:
+ start-addr:
+ maxItems: 1
+ description: Memory start address (32 bits)
+
+ size:
+ maxItems: 1
+ description: size of the I/O MMU block (32 bits)
+
+ iova-align:
+ minItems: 2
+ maxItems: 2
+ description: DMA address alignment of the mapped memory (64 bits)
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ smmu_lpae {
+ compatible = "hisilicon,smmu-lpae";
+
+ iommu_info {
+ start-addr = <0x40000>;
+ size = <0xbffc0000>;
+ iova-align = <0x0 0x8000>;
+ };
+ };
--
2.26.2
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 14/16] dt: add an spec for the Kirin36x0 SMMU
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, Joerg Roedel, Rob Herring, iommu,
devicetree, linux-kernel
Describe the properties expected by the IOMMU driver used on
Hikey960 and Hikey970 boards.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
.../iommu/hisilicon,kirin36x0-smmu.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
diff --git a/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml b/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
new file mode 100644
index 000000000000..ec4c98faf3a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/hisilicon,kirin36x0-smmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon support for HI3660/HI3670 SMMU
+
+maintainers:
+ - Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+
+description: |+
+ Huawei's Hisilicon Kirin 3660/3670 contains a System MMU that enables
+ scattered physical memory chunks visible as a contiguous region to
+ DMA-capable peripheral devices like GPU and ISP.
+
+ The IOMMU domains are described via iommu_info settings.
+
+properties:
+ compatible:
+ const: hisilicon,hisi-smmu-lpae
+
+ iommu_info:
+ type: object
+
+ properties:
+ start-addr:
+ maxItems: 1
+ description: Memory start address (32 bits)
+
+ size:
+ maxItems: 1
+ description: size of the I/O MMU block (32 bits)
+
+ iova-align:
+ minItems: 2
+ maxItems: 2
+ description: DMA address alignment of the mapped memory (64 bits)
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ smmu_lpae {
+ compatible = "hisilicon,smmu-lpae";
+
+ iommu_info {
+ start-addr = <0x40000>;
+ size = <0xbffc0000>;
+ iova-align = <0x0 0x8000>;
+ };
+ };
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH 15/16] dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
2020-08-17 7:49 ` Mauro Carvalho Chehab
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devicetree, Manivannan Sadhasivam, Mauro Carvalho Chehab,
linuxarm, Wei Xu, linux-kernel, Rob Herring, John Stultz,
mauro.chehab, linux-arm-kernel
Those boards come with a system IOMMU, which are required
by certain drivers (DRM/KMS).
Add its dependency to Hikey970 device tree bindings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
index a9ad90e769ad..f632b1e0600d 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
@@ -53,6 +53,9 @@ wlan_en: wlan-en-1-8v {
startup-delay-us = <70000>;
enable-active-high;
};
+ smmu_lpae {
+ compatible = "hisilicon,smmu-lpae";
+ };
};
/*
--
2.26.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 63+ messages in thread* [PATCH 15/16] dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, Wei Xu, Rob Herring, linux-arm-kernel,
devicetree, linux-kernel
Those boards come with a system IOMMU, which are required
by certain drivers (DRM/KMS).
Add its dependency to Hikey970 device tree bindings.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
index a9ad90e769ad..f632b1e0600d 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
@@ -53,6 +53,9 @@ wlan_en: wlan-en-1-8v {
startup-delay-us = <70000>;
enable-active-high;
};
+ smmu_lpae {
+ compatible = "hisilicon,smmu-lpae";
+ };
};
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [PATCH 16/16] staging: hikey9xx: add an item about the iommu driver
2020-08-17 7:49 ` Mauro Carvalho Chehab
` (16 preceding siblings ...)
(?)
@ 2020-08-17 7:50 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 7:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
Manivannan Sadhasivam, linux-kernel, devel
This driver doesn't use a too standard DT. Need to
adjust it and the upcoming DRM driver who uses it, in
order to do things on a more standard way.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/staging/hikey9xx/TODO | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/hikey9xx/TODO b/drivers/staging/hikey9xx/TODO
index 65e7996a3066..79f2cc990415 100644
--- a/drivers/staging/hikey9xx/TODO
+++ b/drivers/staging/hikey9xx/TODO
@@ -2,4 +2,5 @@ ToDo list:
- Port other drivers needed by Hikey 960/970;
- Test drivers on Hikey 960;
+- Adjust the iommu driver's code for it to use more standard DT settings;
- Validate device tree bindings.
--
2.26.2
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 7:49 ` Mauro Carvalho Chehab
(?)
@ 2020-08-17 8:21 ` Christoph Hellwig
-1 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 8:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, linuxarm, Wei Xu, linux-kernel, iommu,
Rob Herring, John Stultz, Chenfeng, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
Please don't add iommu drivers to staging, and just work with the
maintainers to properly clean it up.
I also don't think adding a totally out of date not compiling version
is a good idea. Please do a proper rollup, and if required (probably
not in this case), split it into useful chunks.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 8:21 ` Christoph Hellwig
0 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 8:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, linuxarm, mauro.chehab, John Stultz,
Manivannan Sadhasivam, Rob Herring, linux-arm-kernel, Wei Xu,
devicetree, Joerg Roedel, Joerg Roedel, iommu, Chenfeng, devel,
Suzhuangluan, linux-kernel
On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
Please don't add iommu drivers to staging, and just work with the
maintainers to properly clean it up.
I also don't think adding a totally out of date not compiling version
is a good idea. Please do a proper rollup, and if required (probably
not in this case), split it into useful chunks.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 8:21 ` Christoph Hellwig
0 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 8:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Joerg Roedel, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
Please don't add iommu drivers to staging, and just work with the
maintainers to properly clean it up.
I also don't think adding a totally out of date not compiling version
is a good idea. Please do a proper rollup, and if required (probably
not in this case), split it into useful chunks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 8:21 ` Christoph Hellwig
(?)
@ 2020-08-17 9:27 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, linuxarm, Wei Xu, linux-kernel, iommu,
Rob Herring, John Stultz, Chenfeng, mauro.chehab, Suzhuangluan,
linux-arm-kernel
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <hch@infradead.org> escreveu:
> On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
>
> Please don't add iommu drivers to staging, and just work with the
> maintainers to properly clean it up.
I need to start from the original patch in order to preserve its
authorship.
My plan is to work with the iommu subsystem maintainers after
have this (and another pending patch series for DRM) merged.
> I also don't think adding a totally out of date not compiling version
> is a good idea. Please do a proper rollup, and if required (probably
> not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches
on this series ports it to upstream, cleans up the driver and
address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for
the display engine and by the encoding/decoding images via the ISP.
With this series, this driver builds and runs as expected, providing
IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings
require some work, as, instead of using dma-ranges, the DRM driver binds
into it with:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
dpe: dpe@e8600000 {
compatible = "hisilicon,kirin970-dpe";
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
};
In order to properly address it, the best would be to also have the
DRM driver merged upstream, as it relies on it. So, a change in DT will
also mean a change at the way the DRM uses it.
The DRM itself should go via staging, as it has some bugs that I'd
like to fix before moving it to drivers/gpu/drm. Those are more
tricky to solve, as they seem to require using different settings for
some hardware registers, and the downstream driver also have the same
issues. Fixing them will likely require some time.
Thanks,
Mauro
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:27 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Greg Kroah-Hartman, linuxarm, mauro.chehab, John Stultz,
Manivannan Sadhasivam, Rob Herring, linux-arm-kernel, Wei Xu,
devicetree, Joerg Roedel, Joerg Roedel, iommu, Chenfeng, devel,
Suzhuangluan, linux-kernel
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <hch@infradead.org> escreveu:
> On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
>
> Please don't add iommu drivers to staging, and just work with the
> maintainers to properly clean it up.
I need to start from the original patch in order to preserve its
authorship.
My plan is to work with the iommu subsystem maintainers after
have this (and another pending patch series for DRM) merged.
> I also don't think adding a totally out of date not compiling version
> is a good idea. Please do a proper rollup, and if required (probably
> not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches
on this series ports it to upstream, cleans up the driver and
address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for
the display engine and by the encoding/decoding images via the ISP.
With this series, this driver builds and runs as expected, providing
IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings
require some work, as, instead of using dma-ranges, the DRM driver binds
into it with:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
dpe: dpe@e8600000 {
compatible = "hisilicon,kirin970-dpe";
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
};
In order to properly address it, the best would be to also have the
DRM driver merged upstream, as it relies on it. So, a change in DT will
also mean a change at the way the DRM uses it.
The DRM itself should go via staging, as it has some bugs that I'd
like to fix before moving it to drivers/gpu/drm. Those are more
tricky to solve, as they seem to require using different settings for
some hardware registers, and the downstream driver also have the same
issues. Fixing them will likely require some time.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:27 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 9:27 UTC (permalink / raw)
To: Christoph Hellwig
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Joerg Roedel, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
Suzhuangluan, linux-arm-kernel
Hi Christoph,
Em Mon, 17 Aug 2020 09:21:06 +0100
Christoph Hellwig <hch@infradead.org> escreveu:
> On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
>
> Please don't add iommu drivers to staging, and just work with the
> maintainers to properly clean it up.
I need to start from the original patch in order to preserve its
authorship.
My plan is to work with the iommu subsystem maintainers after
have this (and another pending patch series for DRM) merged.
> I also don't think adding a totally out of date not compiling version
> is a good idea. Please do a proper rollup, and if required (probably
> not in this case), split it into useful chunks.
This series make this driver working as expected.
I mean, while patch 01/16 is against Kernel 4.9, the other patches
on this series ports it to upstream, cleans up the driver and
address several issues on it.
This specific IOMMU seems to be an specific part of the SoC dedicated for
the display engine and by the encoding/decoding images via the ISP.
With this series, this driver builds and runs as expected, providing
IOMMU support needed by the upcoming KMS/DRM driver.
The only issue on it (as far as I can tell) is that the DT bindings
require some work, as, instead of using dma-ranges, the DRM driver binds
into it with:
smmu_lpae {
compatible = "hisilicon,smmu-lpae";
};
dpe: dpe@e8600000 {
compatible = "hisilicon,kirin970-dpe";
...
iommu_info {
start-addr = <0x8000>;
size = <0xbfff8000>;
};
};
In order to properly address it, the best would be to also have the
DRM driver merged upstream, as it relies on it. So, a change in DT will
also mean a change at the way the DRM uses it.
The DRM itself should go via staging, as it has some bugs that I'd
like to fix before moving it to drivers/gpu/drm. Those are more
tricky to solve, as they seem to require using different settings for
some hardware registers, and the downstream driver also have the same
issues. Fixing them will likely require some time.
Thanks,
Mauro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 9:27 ` Mauro Carvalho Chehab
(?)
@ 2020-08-17 9:31 ` Christoph Hellwig
-1 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 9:31 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, linuxarm, Wei Xu, linux-kernel,
Christoph Hellwig, iommu, Rob Herring, John Stultz, Chenfeng,
mauro.chehab, Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> I need to start from the original patch in order to preserve its
> authorship.
Nom you don't. We have plenty of example where people submit code
written by other, especially when it is significantly reworked,
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
Please submit it to the iommu list directly. Seriously - you did not
even cc the relevant list, and you don't even have a comment from
the maintainers. Don't do this kind of crap.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:31 ` Christoph Hellwig
0 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 9:31 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Christoph Hellwig, Greg Kroah-Hartman, linuxarm, mauro.chehab,
John Stultz, Manivannan Sadhasivam, Rob Herring, linux-arm-kernel,
Wei Xu, devicetree, Joerg Roedel, Joerg Roedel, iommu, Chenfeng,
devel, Suzhuangluan, linux-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> I need to start from the original patch in order to preserve its
> authorship.
Nom you don't. We have plenty of example where people submit code
written by other, especially when it is significantly reworked,
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
Please submit it to the iommu list directly. Seriously - you did not
even cc the relevant list, and you don't even have a comment from
the maintainers. Don't do this kind of crap.
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:31 ` Christoph Hellwig
0 siblings, 0 replies; 63+ messages in thread
From: Christoph Hellwig @ 2020-08-17 9:31 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Joerg Roedel, linuxarm, Wei Xu, linux-kernel,
Christoph Hellwig, iommu, Rob Herring, John Stultz, Chenfeng,
mauro.chehab, Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> I need to start from the original patch in order to preserve its
> authorship.
Nom you don't. We have plenty of example where people submit code
written by other, especially when it is significantly reworked,
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
Please submit it to the iommu list directly. Seriously - you did not
even cc the relevant list, and you don't even have a comment from
the maintainers. Don't do this kind of crap.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 9:27 ` Mauro Carvalho Chehab
(?)
@ 2020-08-17 9:37 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 9:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
linuxarm, Wei Xu, linux-kernel, Christoph Hellwig, iommu,
Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> Hi Christoph,
>
> Em Mon, 17 Aug 2020 09:21:06 +0100
> Christoph Hellwig <hch@infradead.org> escreveu:
>
> > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> >
> > Please don't add iommu drivers to staging, and just work with the
> > maintainers to properly clean it up.
>
> I need to start from the original patch in order to preserve its
> authorship.
>
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
>
> > I also don't think adding a totally out of date not compiling version
> > is a good idea. Please do a proper rollup, and if required (probably
> > not in this case), split it into useful chunks.
>
> This series make this driver working as expected.
>
> I mean, while patch 01/16 is against Kernel 4.9, the other patches
> on this series ports it to upstream, cleans up the driver and
> address several issues on it.
>
> This specific IOMMU seems to be an specific part of the SoC dedicated for
> the display engine and by the encoding/decoding images via the ISP.
> With this series, this driver builds and runs as expected, providing
> IOMMU support needed by the upcoming KMS/DRM driver.
>
> The only issue on it (as far as I can tell) is that the DT bindings
> require some work, as, instead of using dma-ranges, the DRM driver binds
> into it with:
>
> smmu_lpae {
> compatible = "hisilicon,smmu-lpae";
> };
>
> dpe: dpe@e8600000 {
> compatible = "hisilicon,kirin970-dpe";
> ...
> iommu_info {
> start-addr = <0x8000>;
> size = <0xbfff8000>;
> };
> };
>
> In order to properly address it, the best would be to also have the
> DRM driver merged upstream, as it relies on it. So, a change in DT will
> also mean a change at the way the DRM uses it.
>
> The DRM itself should go via staging, as it has some bugs that I'd
> like to fix before moving it to drivers/gpu/drm. Those are more
> tricky to solve, as they seem to require using different settings for
> some hardware registers, and the downstream driver also have the same
> issues. Fixing them will likely require some time.
DRM drivers can't go through staging unless you get the DRM developers
to agree with it, and last I heard, they were strongly against it.
It's _always_ faster to just do the work out-of-tree for a week or so
and then merge it correctly to the proper part of the kernel tree. I'd
recommend doing that here for the iommu driver, as well as the DRM
driver.
There's no issues with authorship and the like, just properly attribute
it when you submit it and you are fine.
Again, merging in staging always takes more work and energy, don't do it
unless there is no other way.
thanks,
greg k-h
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:37 ` Greg Kroah-Hartman
0 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 9:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Christoph Hellwig, devel, devicetree, Joerg Roedel,
Manivannan Sadhasivam, Joerg Roedel, linuxarm, Wei Xu,
linux-kernel, iommu, Rob Herring, John Stultz, Chenfeng,
mauro.chehab, Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> Hi Christoph,
>
> Em Mon, 17 Aug 2020 09:21:06 +0100
> Christoph Hellwig <hch@infradead.org> escreveu:
>
> > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> >
> > Please don't add iommu drivers to staging, and just work with the
> > maintainers to properly clean it up.
>
> I need to start from the original patch in order to preserve its
> authorship.
>
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
>
> > I also don't think adding a totally out of date not compiling version
> > is a good idea. Please do a proper rollup, and if required (probably
> > not in this case), split it into useful chunks.
>
> This series make this driver working as expected.
>
> I mean, while patch 01/16 is against Kernel 4.9, the other patches
> on this series ports it to upstream, cleans up the driver and
> address several issues on it.
>
> This specific IOMMU seems to be an specific part of the SoC dedicated for
> the display engine and by the encoding/decoding images via the ISP.
> With this series, this driver builds and runs as expected, providing
> IOMMU support needed by the upcoming KMS/DRM driver.
>
> The only issue on it (as far as I can tell) is that the DT bindings
> require some work, as, instead of using dma-ranges, the DRM driver binds
> into it with:
>
> smmu_lpae {
> compatible = "hisilicon,smmu-lpae";
> };
>
> dpe: dpe@e8600000 {
> compatible = "hisilicon,kirin970-dpe";
> ...
> iommu_info {
> start-addr = <0x8000>;
> size = <0xbfff8000>;
> };
> };
>
> In order to properly address it, the best would be to also have the
> DRM driver merged upstream, as it relies on it. So, a change in DT will
> also mean a change at the way the DRM uses it.
>
> The DRM itself should go via staging, as it has some bugs that I'd
> like to fix before moving it to drivers/gpu/drm. Those are more
> tricky to solve, as they seem to require using different settings for
> some hardware registers, and the downstream driver also have the same
> issues. Fixing them will likely require some time.
DRM drivers can't go through staging unless you get the DRM developers
to agree with it, and last I heard, they were strongly against it.
It's _always_ faster to just do the work out-of-tree for a week or so
and then merge it correctly to the proper part of the kernel tree. I'd
recommend doing that here for the iommu driver, as well as the DRM
driver.
There's no issues with authorship and the like, just properly attribute
it when you submit it and you are fine.
Again, merging in staging always takes more work and energy, don't do it
unless there is no other way.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 9:37 ` Greg Kroah-Hartman
0 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 9:37 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Joerg Roedel, linuxarm, Wei Xu, linux-kernel, Christoph Hellwig,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> Hi Christoph,
>
> Em Mon, 17 Aug 2020 09:21:06 +0100
> Christoph Hellwig <hch@infradead.org> escreveu:
>
> > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> >
> > Please don't add iommu drivers to staging, and just work with the
> > maintainers to properly clean it up.
>
> I need to start from the original patch in order to preserve its
> authorship.
>
> My plan is to work with the iommu subsystem maintainers after
> have this (and another pending patch series for DRM) merged.
>
> > I also don't think adding a totally out of date not compiling version
> > is a good idea. Please do a proper rollup, and if required (probably
> > not in this case), split it into useful chunks.
>
> This series make this driver working as expected.
>
> I mean, while patch 01/16 is against Kernel 4.9, the other patches
> on this series ports it to upstream, cleans up the driver and
> address several issues on it.
>
> This specific IOMMU seems to be an specific part of the SoC dedicated for
> the display engine and by the encoding/decoding images via the ISP.
> With this series, this driver builds and runs as expected, providing
> IOMMU support needed by the upcoming KMS/DRM driver.
>
> The only issue on it (as far as I can tell) is that the DT bindings
> require some work, as, instead of using dma-ranges, the DRM driver binds
> into it with:
>
> smmu_lpae {
> compatible = "hisilicon,smmu-lpae";
> };
>
> dpe: dpe@e8600000 {
> compatible = "hisilicon,kirin970-dpe";
> ...
> iommu_info {
> start-addr = <0x8000>;
> size = <0xbfff8000>;
> };
> };
>
> In order to properly address it, the best would be to also have the
> DRM driver merged upstream, as it relies on it. So, a change in DT will
> also mean a change at the way the DRM uses it.
>
> The DRM itself should go via staging, as it has some bugs that I'd
> like to fix before moving it to drivers/gpu/drm. Those are more
> tricky to solve, as they seem to require using different settings for
> some hardware registers, and the downstream driver also have the same
> issues. Fixing them will likely require some time.
DRM drivers can't go through staging unless you get the DRM developers
to agree with it, and last I heard, they were strongly against it.
It's _always_ faster to just do the work out-of-tree for a week or so
and then merge it correctly to the proper part of the kernel tree. I'd
recommend doing that here for the iommu driver, as well as the DRM
driver.
There's no issues with authorship and the like, just properly attribute
it when you submit it and you are fine.
Again, merging in staging always takes more work and energy, don't do it
unless there is no other way.
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 9:37 ` Greg Kroah-Hartman
(?)
@ 2020-08-17 10:46 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 10:46 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
linuxarm, Wei Xu, linux-kernel, Christoph Hellwig, iommu,
Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
Em Mon, 17 Aug 2020 11:37:03 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Christoph,
> >
> > Em Mon, 17 Aug 2020 09:21:06 +0100
> > Christoph Hellwig <hch@infradead.org> escreveu:
> >
> > > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> > >
> > > Please don't add iommu drivers to staging, and just work with the
> > > maintainers to properly clean it up.
> >
> > I need to start from the original patch in order to preserve its
> > authorship.
> >
> > My plan is to work with the iommu subsystem maintainers after
> > have this (and another pending patch series for DRM) merged.
> >
> > > I also don't think adding a totally out of date not compiling version
> > > is a good idea. Please do a proper rollup, and if required (probably
> > > not in this case), split it into useful chunks.
> >
> > This series make this driver working as expected.
> >
> > I mean, while patch 01/16 is against Kernel 4.9, the other patches
> > on this series ports it to upstream, cleans up the driver and
> > address several issues on it.
> >
> > This specific IOMMU seems to be an specific part of the SoC dedicated for
> > the display engine and by the encoding/decoding images via the ISP.
> > With this series, this driver builds and runs as expected, providing
> > IOMMU support needed by the upcoming KMS/DRM driver.
> >
> > The only issue on it (as far as I can tell) is that the DT bindings
> > require some work, as, instead of using dma-ranges, the DRM driver binds
> > into it with:
> >
> > smmu_lpae {
> > compatible = "hisilicon,smmu-lpae";
> > };
> >
> > dpe: dpe@e8600000 {
> > compatible = "hisilicon,kirin970-dpe";
> > ...
> > iommu_info {
> > start-addr = <0x8000>;
> > size = <0xbfff8000>;
> > };
> > };
> >
> > In order to properly address it, the best would be to also have the
> > DRM driver merged upstream, as it relies on it. So, a change in DT will
> > also mean a change at the way the DRM uses it.
> >
> > The DRM itself should go via staging, as it has some bugs that I'd
> > like to fix before moving it to drivers/gpu/drm. Those are more
> > tricky to solve, as they seem to require using different settings for
> > some hardware registers, and the downstream driver also have the same
> > issues. Fixing them will likely require some time.
>
> DRM drivers can't go through staging unless you get the DRM developers
> to agree with it, and last I heard, they were strongly against it.
Ok, I'll ping them. There's already a thread I opened at the DRM devel
ML about this driver. IMHO, there's nothing on this specific driver
that would prevent having it on staging for a while, until the two
or three remaining bugs gets fixed.
On the other hand, the code already follows the current DRM policies,
as far as I can tell. The bugs are related to some specific register
settings that would need to be better tuned (and maybe some delay
when waiting for EDID data at the already-existing adv7535 driver).
Maybe it would be preferred to have it at drivers/gpu even with
such bugs.
> It's _always_ faster to just do the work out-of-tree for a week or so
> and then merge it correctly to the proper part of the kernel tree. I'd
> recommend doing that here for the iommu driver, as well as the DRM
> driver.
It is OK for me to working for a week or so, until the iommu people
become happy with that.
The main reason of submitting via staging is that I need to preserve
the patch that added this driver as-is, in order to preserve its
SoB and not causing legal issues.
It it is OK for iommu to accept a submission like that, I can
re-submit it, doing the changes at drivers/iommu.
If not, besides this series, the only alternative I can think of is to
merge it first at the staging, with the incremental changes, and with
a final patch moving the code out of staging.
Thanks,
Mauro
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 10:46 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 10:46 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Christoph Hellwig, devel, devicetree, Joerg Roedel,
Manivannan Sadhasivam, Joerg Roedel, linuxarm, Wei Xu,
linux-kernel, iommu, Rob Herring, John Stultz, Chenfeng,
mauro.chehab, Suzhuangluan, linux-arm-kernel
Em Mon, 17 Aug 2020 11:37:03 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Christoph,
> >
> > Em Mon, 17 Aug 2020 09:21:06 +0100
> > Christoph Hellwig <hch@infradead.org> escreveu:
> >
> > > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> > >
> > > Please don't add iommu drivers to staging, and just work with the
> > > maintainers to properly clean it up.
> >
> > I need to start from the original patch in order to preserve its
> > authorship.
> >
> > My plan is to work with the iommu subsystem maintainers after
> > have this (and another pending patch series for DRM) merged.
> >
> > > I also don't think adding a totally out of date not compiling version
> > > is a good idea. Please do a proper rollup, and if required (probably
> > > not in this case), split it into useful chunks.
> >
> > This series make this driver working as expected.
> >
> > I mean, while patch 01/16 is against Kernel 4.9, the other patches
> > on this series ports it to upstream, cleans up the driver and
> > address several issues on it.
> >
> > This specific IOMMU seems to be an specific part of the SoC dedicated for
> > the display engine and by the encoding/decoding images via the ISP.
> > With this series, this driver builds and runs as expected, providing
> > IOMMU support needed by the upcoming KMS/DRM driver.
> >
> > The only issue on it (as far as I can tell) is that the DT bindings
> > require some work, as, instead of using dma-ranges, the DRM driver binds
> > into it with:
> >
> > smmu_lpae {
> > compatible = "hisilicon,smmu-lpae";
> > };
> >
> > dpe: dpe@e8600000 {
> > compatible = "hisilicon,kirin970-dpe";
> > ...
> > iommu_info {
> > start-addr = <0x8000>;
> > size = <0xbfff8000>;
> > };
> > };
> >
> > In order to properly address it, the best would be to also have the
> > DRM driver merged upstream, as it relies on it. So, a change in DT will
> > also mean a change at the way the DRM uses it.
> >
> > The DRM itself should go via staging, as it has some bugs that I'd
> > like to fix before moving it to drivers/gpu/drm. Those are more
> > tricky to solve, as they seem to require using different settings for
> > some hardware registers, and the downstream driver also have the same
> > issues. Fixing them will likely require some time.
>
> DRM drivers can't go through staging unless you get the DRM developers
> to agree with it, and last I heard, they were strongly against it.
Ok, I'll ping them. There's already a thread I opened at the DRM devel
ML about this driver. IMHO, there's nothing on this specific driver
that would prevent having it on staging for a while, until the two
or three remaining bugs gets fixed.
On the other hand, the code already follows the current DRM policies,
as far as I can tell. The bugs are related to some specific register
settings that would need to be better tuned (and maybe some delay
when waiting for EDID data at the already-existing adv7535 driver).
Maybe it would be preferred to have it at drivers/gpu even with
such bugs.
> It's _always_ faster to just do the work out-of-tree for a week or so
> and then merge it correctly to the proper part of the kernel tree. I'd
> recommend doing that here for the iommu driver, as well as the DRM
> driver.
It is OK for me to working for a week or so, until the iommu people
become happy with that.
The main reason of submitting via staging is that I need to preserve
the patch that added this driver as-is, in order to preserve its
SoB and not causing legal issues.
It it is OK for iommu to accept a submission like that, I can
re-submit it, doing the changes at drivers/iommu.
If not, besides this series, the only alternative I can think of is to
merge it first at the staging, with the incremental changes, and with
a final patch moving the code out of staging.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 10:46 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-17 10:46 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Joerg Roedel, linuxarm, Wei Xu, linux-kernel, Christoph Hellwig,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
Em Mon, 17 Aug 2020 11:37:03 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> escreveu:
> On Mon, Aug 17, 2020 at 11:27:25AM +0200, Mauro Carvalho Chehab wrote:
> > Hi Christoph,
> >
> > Em Mon, 17 Aug 2020 09:21:06 +0100
> > Christoph Hellwig <hch@infradead.org> escreveu:
> >
> > > On Mon, Aug 17, 2020 at 09:49:59AM +0200, 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.
> > >
> > > Please don't add iommu drivers to staging, and just work with the
> > > maintainers to properly clean it up.
> >
> > I need to start from the original patch in order to preserve its
> > authorship.
> >
> > My plan is to work with the iommu subsystem maintainers after
> > have this (and another pending patch series for DRM) merged.
> >
> > > I also don't think adding a totally out of date not compiling version
> > > is a good idea. Please do a proper rollup, and if required (probably
> > > not in this case), split it into useful chunks.
> >
> > This series make this driver working as expected.
> >
> > I mean, while patch 01/16 is against Kernel 4.9, the other patches
> > on this series ports it to upstream, cleans up the driver and
> > address several issues on it.
> >
> > This specific IOMMU seems to be an specific part of the SoC dedicated for
> > the display engine and by the encoding/decoding images via the ISP.
> > With this series, this driver builds and runs as expected, providing
> > IOMMU support needed by the upcoming KMS/DRM driver.
> >
> > The only issue on it (as far as I can tell) is that the DT bindings
> > require some work, as, instead of using dma-ranges, the DRM driver binds
> > into it with:
> >
> > smmu_lpae {
> > compatible = "hisilicon,smmu-lpae";
> > };
> >
> > dpe: dpe@e8600000 {
> > compatible = "hisilicon,kirin970-dpe";
> > ...
> > iommu_info {
> > start-addr = <0x8000>;
> > size = <0xbfff8000>;
> > };
> > };
> >
> > In order to properly address it, the best would be to also have the
> > DRM driver merged upstream, as it relies on it. So, a change in DT will
> > also mean a change at the way the DRM uses it.
> >
> > The DRM itself should go via staging, as it has some bugs that I'd
> > like to fix before moving it to drivers/gpu/drm. Those are more
> > tricky to solve, as they seem to require using different settings for
> > some hardware registers, and the downstream driver also have the same
> > issues. Fixing them will likely require some time.
>
> DRM drivers can't go through staging unless you get the DRM developers
> to agree with it, and last I heard, they were strongly against it.
Ok, I'll ping them. There's already a thread I opened at the DRM devel
ML about this driver. IMHO, there's nothing on this specific driver
that would prevent having it on staging for a while, until the two
or three remaining bugs gets fixed.
On the other hand, the code already follows the current DRM policies,
as far as I can tell. The bugs are related to some specific register
settings that would need to be better tuned (and maybe some delay
when waiting for EDID data at the already-existing adv7535 driver).
Maybe it would be preferred to have it at drivers/gpu even with
such bugs.
> It's _always_ faster to just do the work out-of-tree for a week or so
> and then merge it correctly to the proper part of the kernel tree. I'd
> recommend doing that here for the iommu driver, as well as the DRM
> driver.
It is OK for me to working for a week or so, until the iommu people
become happy with that.
The main reason of submitting via staging is that I need to preserve
the patch that added this driver as-is, in order to preserve its
SoB and not causing legal issues.
It it is OK for iommu to accept a submission like that, I can
re-submit it, doing the changes at drivers/iommu.
If not, besides this series, the only alternative I can think of is to
merge it first at the staging, with the incremental changes, and with
a final patch moving the code out of staging.
Thanks,
Mauro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 10:46 ` Mauro Carvalho Chehab
(?)
@ 2020-08-17 10:53 ` Greg Kroah-Hartman
-1 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 10:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
linuxarm, Wei Xu, linux-kernel, Christoph Hellwig, iommu,
Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 12:46:17PM +0200, Mauro Carvalho Chehab wrote:
> The main reason of submitting via staging is that I need to preserve
> the patch that added this driver as-is, in order to preserve its
> SoB and not causing legal issues.
>
> It it is OK for iommu to accept a submission like that, I can
> re-submit it, doing the changes at drivers/iommu.
You can always do this just fine, as one single patch. You do know
about the co-developed-by: line, right?
thanks,
greg k-h
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 10:53 ` Greg Kroah-Hartman
0 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 10:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Joerg Roedel, linuxarm, Wei Xu, linux-kernel, Christoph Hellwig,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 12:46:17PM +0200, Mauro Carvalho Chehab wrote:
> The main reason of submitting via staging is that I need to preserve
> the patch that added this driver as-is, in order to preserve its
> SoB and not causing legal issues.
>
> It it is OK for iommu to accept a submission like that, I can
> re-submit it, doing the changes at drivers/iommu.
You can always do this just fine, as one single patch. You do know
about the co-developed-by: line, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 10:53 ` Greg Kroah-Hartman
0 siblings, 0 replies; 63+ messages in thread
From: Greg Kroah-Hartman @ 2020-08-17 10:53 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Joerg Roedel, linuxarm, Wei Xu, linux-kernel, Christoph Hellwig,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 12:46:17PM +0200, Mauro Carvalho Chehab wrote:
> The main reason of submitting via staging is that I need to preserve
> the patch that added this driver as-is, in order to preserve its
> SoB and not causing legal issues.
>
> It it is OK for iommu to accept a submission like that, I can
> re-submit it, doing the changes at drivers/iommu.
You can always do this just fine, as one single patch. You do know
about the co-developed-by: line, right?
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 10:53 ` Greg Kroah-Hartman
(?)
@ 2020-08-17 12:59 ` Joerg Roedel
-1 siblings, 0 replies; 63+ messages in thread
From: Joerg Roedel @ 2020-08-17 12:59 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Manivannan Sadhasivam, Greg Kroah-Hartman,
linuxarm, Wei Xu, linux-kernel, Christoph Hellwig, iommu,
Rob Herring, John Stultz, Chenfeng, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Mon, Aug 17, 2020 at 12:53:45PM +0200, Greg Kroah-Hartman wrote:
> You can always do this just fine, as one single patch. You do know
> about the co-developed-by: line, right?
Agreed. Please keep the main iommu driver in one patch and use
co-developed-by. This makes it easier for me to review it and provide
feedback. And please Cc me on the whole patch-set for v2.
Thanks,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 12:59 ` Joerg Roedel
0 siblings, 0 replies; 63+ messages in thread
From: Joerg Roedel @ 2020-08-17 12:59 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, devel, devicetree, Manivannan Sadhasivam,
Chenfeng, Joerg Roedel, linuxarm, Wei Xu, linux-kernel,
Christoph Hellwig, iommu, Rob Herring, John Stultz, mauro.chehab,
Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 12:53:45PM +0200, Greg Kroah-Hartman wrote:
> You can always do this just fine, as one single patch. You do know
> about the co-developed-by: line, right?
Agreed. Please keep the main iommu driver in one patch and use
co-developed-by. This makes it easier for me to review it and provide
feedback. And please Cc me on the whole patch-set for v2.
Thanks,
Joerg
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-17 12:59 ` Joerg Roedel
0 siblings, 0 replies; 63+ messages in thread
From: Joerg Roedel @ 2020-08-17 12:59 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Manivannan Sadhasivam, Greg Kroah-Hartman,
Joerg Roedel, linuxarm, Wei Xu, linux-kernel, Christoph Hellwig,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
Suzhuangluan, linux-arm-kernel
On Mon, Aug 17, 2020 at 12:53:45PM +0200, Greg Kroah-Hartman wrote:
> You can always do this just fine, as one single patch. You do know
> about the co-developed-by: line, right?
Agreed. Please keep the main iommu driver in one patch and use
co-developed-by. This makes it easier for me to review it and provide
feedback. And please Cc me on the whole patch-set for v2.
Thanks,
Joerg
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-17 7:49 ` Mauro Carvalho Chehab
(?)
@ 2020-08-18 14:47 ` Robin Murphy
-1 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 14:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Suzhuangluan, linuxarm, Wei Xu, linux-kernel, iommu, Rob Herring,
John Stultz, mauro.chehab, linux-arm-kernel
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 ;)
Robin.
> dt: add an spec for the Kirin36x0 SMMU
> dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
> staging: hikey9xx: add an item about the iommu driver
>
> .../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++
> .../boot/dts/hisilicon/hi3670-hikey970.dts | 3 +
> drivers/staging/hikey9xx/Kconfig | 9 +
> drivers/staging/hikey9xx/Makefile | 1 +
> drivers/staging/hikey9xx/TODO | 1 +
> drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++
> drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++
> 7 files changed, 913 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 14:47 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 14:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
linuxarm, Wei Xu, linux-kernel, iommu, Rob Herring, John Stultz,
mauro.chehab, Suzhuangluan, linux-arm-kernel
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 ;)
Robin.
> dt: add an spec for the Kirin36x0 SMMU
> dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
> staging: hikey9xx: add an item about the iommu driver
>
> .../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++
> .../boot/dts/hisilicon/hi3670-hikey970.dts | 3 +
> drivers/staging/hikey9xx/Kconfig | 9 +
> drivers/staging/hikey9xx/Makefile | 1 +
> drivers/staging/hikey9xx/TODO | 1 +
> drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++
> drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++
> 7 files changed, 913 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 14:47 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 14:47 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Greg Kroah-Hartman
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam, Chenfeng,
Suzhuangluan, linuxarm, Wei Xu, linux-kernel, iommu, Rob Herring,
John Stultz, mauro.chehab, linux-arm-kernel
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 ;)
Robin.
> dt: add an spec for the Kirin36x0 SMMU
> dt: hi3670-hikey970.dts: load the SMMU driver on Hikey970
> staging: hikey9xx: add an item about the iommu driver
>
> .../iommu/hisilicon,kirin36x0-smmu.yaml | 55 ++
> .../boot/dts/hisilicon/hi3670-hikey970.dts | 3 +
> drivers/staging/hikey9xx/Kconfig | 9 +
> drivers/staging/hikey9xx/Makefile | 1 +
> drivers/staging/hikey9xx/TODO | 1 +
> drivers/staging/hikey9xx/hisi_smmu.h | 196 ++++++
> drivers/staging/hikey9xx/hisi_smmu_lpae.c | 648 ++++++++++++++++++
> 7 files changed, 913 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iommu/hisilicon,kirin36x0-smmu.yaml
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu.h
> create mode 100644 drivers/staging/hikey9xx/hisi_smmu_lpae.c
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-18 14:47 ` Robin Murphy
(?)
@ 2020-08-18 15:29 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-18 15:29 UTC (permalink / raw)
To: Robin Murphy
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
linux-arm-kernel
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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 15:29 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-18 15:29 UTC (permalink / raw)
To: Robin Murphy
Cc: Greg Kroah-Hartman, devel, devicetree, Joerg Roedel,
Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 15:29 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-18 15:29 UTC (permalink / raw)
To: Robin Murphy
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
linux-arm-kernel
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-18 15:29 ` Mauro Carvalho Chehab
(?)
@ 2020-08-18 16:26 ` Robin Murphy
-1 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 16:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
linux-arm-kernel
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@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.
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.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 16:26 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 16:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Greg Kroah-Hartman, devel, devicetree, Joerg Roedel,
Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, mauro.chehab, Suzhuangluan,
linux-arm-kernel
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@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.
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.
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 16:26 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-18 16:26 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: devel, devicetree, Joerg Roedel, Manivannan Sadhasivam,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, linux-kernel,
iommu, Rob Herring, John Stultz, Chenfeng, mauro.chehab,
linux-arm-kernel
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@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.
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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-18 16:26 ` Robin Murphy
(?)
@ 2020-08-18 22:02 ` John Stultz
-1 siblings, 0 replies; 63+ messages in thread
From: John Stultz @ 2020-08-18 22:02 UTC (permalink / raw)
To: Robin Murphy
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, lkml, iommu,
Rob Herring, Chenfeng, mauro.chehab, linux-arm-kernel
On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 22:02 ` John Stultz
0 siblings, 0 replies; 63+ messages in thread
From: John Stultz @ 2020-08-18 22:02 UTC (permalink / raw)
To: Robin Murphy
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu,
lkml, iommu, Rob Herring, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-18 22:02 ` John Stultz
0 siblings, 0 replies; 63+ messages in thread
From: John Stultz @ 2020-08-18 22:02 UTC (permalink / raw)
To: Robin Murphy
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, lkml, iommu,
Rob Herring, Chenfeng, mauro.chehab, linux-arm-kernel
On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-18 22:02 ` John Stultz
(?)
@ 2020-08-19 10:12 ` Robin Murphy
-1 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 10:12 UTC (permalink / raw)
To: John Stultz
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, lkml, iommu,
Rob Herring, Chenfeng, mauro.chehab, linux-arm-kernel
On 2020-08-18 23:02, John Stultz wrote:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Indeed, I'd assume it's possible to leave the MMU off and just use CMA
buffers instead, but wiring it up properly without the downstream
mis-design should be pretty clean, so maybe that could ultimately be
shared with 960 too (assuming the hardware isn't wildly dissimilar).
I notice there's already a whole load of MMU configuration hard-coded
into the DRM driver - does iommu_info even need to be in the DT, or
could that also be decided directly by the driver? (Most other MMU-aware
DRM drivers seem to hard-code their drm_mm dimensions.) I can't imagine
the *virtual* address space limits need to vary on a per-board basis,
and they could easily be tied to the compatible if they legitimately
differ across SoCs and a simple lowest-common-denominator approach
wouldn't suffice for whatever reason.
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 10:12 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 10:12 UTC (permalink / raw)
To: John Stultz
Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu,
lkml, iommu, Rob Herring, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On 2020-08-18 23:02, John Stultz wrote:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Indeed, I'd assume it's possible to leave the MMU off and just use CMA
buffers instead, but wiring it up properly without the downstream
mis-design should be pretty clean, so maybe that could ultimately be
shared with 960 too (assuming the hardware isn't wildly dissimilar).
I notice there's already a whole load of MMU configuration hard-coded
into the DRM driver - does iommu_info even need to be in the DT, or
could that also be decided directly by the driver? (Most other MMU-aware
DRM drivers seem to hard-code their drm_mm dimensions.) I can't imagine
the *virtual* address space limits need to vary on a per-board basis,
and they could easily be tied to the compatible if they legitimately
differ across SoCs and a simple lowest-common-denominator approach
wouldn't suffice for whatever reason.
Robin.
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 10:12 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 10:12 UTC (permalink / raw)
To: John Stultz
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Mauro Carvalho Chehab,
Greg Kroah-Hartman, Suzhuangluan, linuxarm, Wei Xu, lkml, iommu,
Rob Herring, Chenfeng, mauro.chehab, linux-arm-kernel
On 2020-08-18 23:02, John Stultz wrote:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Indeed, I'd assume it's possible to leave the MMU off and just use CMA
buffers instead, but wiring it up properly without the downstream
mis-design should be pretty clean, so maybe that could ultimately be
shared with 960 too (assuming the hardware isn't wildly dissimilar).
I notice there's already a whole load of MMU configuration hard-coded
into the DRM driver - does iommu_info even need to be in the DT, or
could that also be decided directly by the driver? (Most other MMU-aware
DRM drivers seem to hard-code their drm_mm dimensions.) I can't imagine
the *virtual* address space limits need to vary on a per-board basis,
and they could easily be tied to the compatible if they legitimately
differ across SoCs and a simple lowest-common-denominator approach
wouldn't suffice for whatever reason.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread
* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-18 22:02 ` John Stultz
(?)
@ 2020-08-19 10:28 ` Mauro Carvalho Chehab
-1 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-19 10:28 UTC (permalink / raw)
To: John Stultz, Robin Murphy
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Greg Kroah-Hartman,
Suzhuangluan, linuxarm, Wei Xu, lkml, iommu, Rob Herring,
Chenfeng, mauro.chehab, linux-arm-kernel
Em Tue, 18 Aug 2020 15:02:54 -0700
John Stultz <john.stultz@linaro.org> escreveu:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Funny... while the Hikey 970 DRM driver has such IOMMU code, it
doesn't actually use it!
The driver has a function called hisi_dss_smmu_config() with
sets the registers on a different way in order to use IOMMU
or not, at the hisi_fb_pan_display() function. It can also
use a mode called "afbcd".
Well, this function sets both to false:
bool afbcd = false;
bool mmu_enable = false;
I ended commenting out the code which depends at the iommu
driver and everything is working as before.
So, I'll just forget about this iommu driver, as we can live
without that.
For now, I'll keep the mmu code there commented out, as
it could be useful on a future port for it to use io-pgtable.
-
Robin,
Can the Panfrost driver use io-pgtable while the KMS driver
won't be using it? Or this would cause it to not work?
My end goal here is to be able to test the Panfrost driver ;-)
Thanks,
Mauro
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 10:28 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-19 10:28 UTC (permalink / raw)
To: John Stultz, Robin Murphy
Cc: Greg Kroah-Hartman, driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu,
lkml, iommu, Rob Herring, mauro.chehab, Suzhuangluan,
linux-arm-kernel
Em Tue, 18 Aug 2020 15:02:54 -0700
John Stultz <john.stultz@linaro.org> escreveu:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Funny... while the Hikey 970 DRM driver has such IOMMU code, it
doesn't actually use it!
The driver has a function called hisi_dss_smmu_config() with
sets the registers on a different way in order to use IOMMU
or not, at the hisi_fb_pan_display() function. It can also
use a mode called "afbcd".
Well, this function sets both to false:
bool afbcd = false;
bool mmu_enable = false;
I ended commenting out the code which depends at the iommu
driver and everything is working as before.
So, I'll just forget about this iommu driver, as we can live
without that.
For now, I'll keep the mmu code there commented out, as
it could be useful on a future port for it to use io-pgtable.
-
Robin,
Can the Panfrost driver use io-pgtable while the KMS driver
won't be using it? Or this would cause it to not work?
My end goal here is to be able to test the Panfrost driver ;-)
Thanks,
Mauro
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 10:28 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 63+ messages in thread
From: Mauro Carvalho Chehab @ 2020-08-19 10:28 UTC (permalink / raw)
To: John Stultz, Robin Murphy
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Greg Kroah-Hartman,
Suzhuangluan, linuxarm, Wei Xu, lkml, iommu, Rob Herring,
Chenfeng, mauro.chehab, linux-arm-kernel
Em Tue, 18 Aug 2020 15:02:54 -0700
John Stultz <john.stultz@linaro.org> escreveu:
> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
Funny... while the Hikey 970 DRM driver has such IOMMU code, it
doesn't actually use it!
The driver has a function called hisi_dss_smmu_config() with
sets the registers on a different way in order to use IOMMU
or not, at the hisi_fb_pan_display() function. It can also
use a mode called "afbcd".
Well, this function sets both to false:
bool afbcd = false;
bool mmu_enable = false;
I ended commenting out the code which depends at the iommu
driver and everything is working as before.
So, I'll just forget about this iommu driver, as we can live
without that.
For now, I'll keep the mmu code there commented out, as
it could be useful on a future port for it to use io-pgtable.
-
Robin,
Can the Panfrost driver use io-pgtable while the KMS driver
won't be using it? Or this would cause it to not work?
My end goal here is to be able to test the Panfrost driver ;-)
Thanks,
Mauro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
2020-08-19 10:28 ` Mauro Carvalho Chehab
(?)
@ 2020-08-19 11:33 ` Robin Murphy
-1 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 11:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, John Stultz
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Greg Kroah-Hartman,
Suzhuangluan, linuxarm, Wei Xu, lkml, iommu, Rob Herring,
Chenfeng, mauro.chehab, linux-arm-kernel
On 2020-08-19 11:28, Mauro Carvalho Chehab wrote:
> Em Tue, 18 Aug 2020 15:02:54 -0700
> John Stultz <john.stultz@linaro.org> escreveu:
>
>> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
>
> Funny... while the Hikey 970 DRM driver has such IOMMU code, it
> doesn't actually use it!
>
> The driver has a function called hisi_dss_smmu_config() with
> sets the registers on a different way in order to use IOMMU
> or not, at the hisi_fb_pan_display() function. It can also
> use a mode called "afbcd".
>
> Well, this function sets both to false:
>
> bool afbcd = false;
> bool mmu_enable = false;
>
> I ended commenting out the code which depends at the iommu
> driver and everything is working as before.
>
> So, I'll just forget about this iommu driver, as we can live
> without that.
>
> For now, I'll keep the mmu code there commented out, as
> it could be useful on a future port for it to use io-pgtable.
>
> -
>
> Robin,
>
> Can the Panfrost driver use io-pgtable while the KMS driver
> won't be using it? Or this would cause it to not work?
>
> My end goal here is to be able to test the Panfrost driver ;-)
Yup, the GPU has its own independent MMU, so Panfrost can import display
buffers regardless of whether they're physically contiguous or not.
Since Mesa master has recently landed AFBC support, there's probably
more immediate benefit in getting that AFBC decoder working before the
display MMU (although ultimately things are likely to work better under
memory pressure if you don't have to rely on CMA, so it should still be
worth coming back to at some point).
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 11:33 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 11:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, John Stultz
Cc: Greg Kroah-Hartman, driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Chenfeng, linuxarm, Wei Xu,
lkml, iommu, Rob Herring, mauro.chehab, Suzhuangluan,
linux-arm-kernel
On 2020-08-19 11:28, Mauro Carvalho Chehab wrote:
> Em Tue, 18 Aug 2020 15:02:54 -0700
> John Stultz <john.stultz@linaro.org> escreveu:
>
>> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
>
> Funny... while the Hikey 970 DRM driver has such IOMMU code, it
> doesn't actually use it!
>
> The driver has a function called hisi_dss_smmu_config() with
> sets the registers on a different way in order to use IOMMU
> or not, at the hisi_fb_pan_display() function. It can also
> use a mode called "afbcd".
>
> Well, this function sets both to false:
>
> bool afbcd = false;
> bool mmu_enable = false;
>
> I ended commenting out the code which depends at the iommu
> driver and everything is working as before.
>
> So, I'll just forget about this iommu driver, as we can live
> without that.
>
> For now, I'll keep the mmu code there commented out, as
> it could be useful on a future port for it to use io-pgtable.
>
> -
>
> Robin,
>
> Can the Panfrost driver use io-pgtable while the KMS driver
> won't be using it? Or this would cause it to not work?
>
> My end goal here is to be able to test the Panfrost driver ;-)
Yup, the GPU has its own independent MMU, so Panfrost can import display
buffers regardless of whether they're physically contiguous or not.
Since Mesa master has recently landed AFBC support, there's probably
more immediate benefit in getting that AFBC decoder working before the
display MMU (although ultimately things are likely to work better under
memory pressure if you don't have to rely on CMA, so it should still be
worth coming back to at some point).
Robin.
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [PATCH 00/16] IOMMU driver for Kirin 960/970
@ 2020-08-19 11:33 ` Robin Murphy
0 siblings, 0 replies; 63+ messages in thread
From: Robin Murphy @ 2020-08-19 11:33 UTC (permalink / raw)
To: Mauro Carvalho Chehab, John Stultz
Cc: driverdevel,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Joerg Roedel, Manivannan Sadhasivam, Greg Kroah-Hartman,
Suzhuangluan, linuxarm, Wei Xu, lkml, iommu, Rob Herring,
Chenfeng, mauro.chehab, linux-arm-kernel
On 2020-08-19 11:28, Mauro Carvalho Chehab wrote:
> Em Tue, 18 Aug 2020 15:02:54 -0700
> John Stultz <john.stultz@linaro.org> escreveu:
>
>> On Tue, Aug 18, 2020 at 9:26 AM Robin Murphy <robin.murphy@arm.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@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).
>
> Funny... while the Hikey 970 DRM driver has such IOMMU code, it
> doesn't actually use it!
>
> The driver has a function called hisi_dss_smmu_config() with
> sets the registers on a different way in order to use IOMMU
> or not, at the hisi_fb_pan_display() function. It can also
> use a mode called "afbcd".
>
> Well, this function sets both to false:
>
> bool afbcd = false;
> bool mmu_enable = false;
>
> I ended commenting out the code which depends at the iommu
> driver and everything is working as before.
>
> So, I'll just forget about this iommu driver, as we can live
> without that.
>
> For now, I'll keep the mmu code there commented out, as
> it could be useful on a future port for it to use io-pgtable.
>
> -
>
> Robin,
>
> Can the Panfrost driver use io-pgtable while the KMS driver
> won't be using it? Or this would cause it to not work?
>
> My end goal here is to be able to test the Panfrost driver ;-)
Yup, the GPU has its own independent MMU, so Panfrost can import display
buffers regardless of whether they're physically contiguous or not.
Since Mesa master has recently landed AFBC support, there's probably
more immediate benefit in getting that AFBC decoder working before the
display MMU (although ultimately things are likely to work better under
memory pressure if you don't have to rely on CMA, so it should still be
worth coming back to at some point).
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 63+ messages in thread