linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
  2019-11-22 23:31 ` [PATCH v2 2/8] iommu: Add DOMAIN_ATTR_SPLIT_TABLES Jordan Crouse
@ 2019-11-22 23:31 ` Jordan Crouse
  2019-12-03 19:14   ` Rob Herring
  2019-11-22 23:31 ` [PATCH v2 3/8] iommu/arm-smmu: Pass io_pgtable_cfg to impl specific init_context Jordan Crouse
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:31 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm, devicetree,
	linux-kernel, Rob Herring, Mark Rutland, Joerg Roedel

Add a compatible string to identify SMMUs that are attached
to Adreno GPU devices that wish to support split pagetables.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 6515dbe..db9f826 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -31,6 +31,12 @@ properties:
               - qcom,sdm845-smmu-v2
           - const: qcom,smmu-v2
 
+      - description: Qcom Adreno GPU SMMU iplementing split pagetables
+        items:
+          - enum:
+              - qcom,adreno-smmu-v2
+          - const: qcom,smmu-v2
+
       - description: Qcom SoCs implementing "arm,mmu-500"
         items:
           - enum:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/8] iommu: Add DOMAIN_ATTR_SPLIT_TABLES
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
@ 2019-11-22 23:31 ` Jordan Crouse
  2019-11-22 23:31 ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Jordan Crouse
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:31 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm,
	Joerg Roedel, linux-kernel

Add a new attribute to enable and query the state of split pagetables
for the domain.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 include/linux/iommu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index f2223cb..18c861e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -126,6 +126,7 @@ enum iommu_attr {
 	DOMAIN_ATTR_FSL_PAMUV1,
 	DOMAIN_ATTR_NESTING,	/* two stages of translation */
 	DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
+	DOMAIN_ATTR_SPLIT_TABLES,
 	DOMAIN_ATTR_MAX,
 };
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/8] iommu/arm-smmu: Pass io_pgtable_cfg to impl specific init_context
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
  2019-11-22 23:31 ` [PATCH v2 2/8] iommu: Add DOMAIN_ATTR_SPLIT_TABLES Jordan Crouse
  2019-11-22 23:31 ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Jordan Crouse
@ 2019-11-22 23:31 ` Jordan Crouse
  2019-11-22 23:31 ` [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations Jordan Crouse
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:31 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm,
	linux-kernel, Joerg Roedel

Pass the propposed io_pgtable_cfg to the implementation specific
init_context() function to give the implementation an opportunity to to
modify it before it gets passed to io-pgtable.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/iommu/arm-smmu-impl.c |  3 ++-
 drivers/iommu/arm-smmu.c      | 11 ++++++-----
 drivers/iommu/arm-smmu.h      |  3 ++-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index b2fe72a..33ed682 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -68,7 +68,8 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
 	return 0;
 }
 
-static int cavium_init_context(struct arm_smmu_domain *smmu_domain)
+static int cavium_init_context(struct arm_smmu_domain *smmu_domain,
+		struct io_pgtable_cfg *pgtbl_cfg)
 {
 	struct cavium_smmu *cs = container_of(smmu_domain->smmu,
 					      struct cavium_smmu, smmu);
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c106406..5c7c32b 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -775,11 +775,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		cfg->asid = cfg->cbndx;
 
 	smmu_domain->smmu = smmu;
-	if (smmu->impl && smmu->impl->init_context) {
-		ret = smmu->impl->init_context(smmu_domain);
-		if (ret)
-			goto out_unlock;
-	}
 
 	pgtbl_cfg = (struct io_pgtable_cfg) {
 		.pgsize_bitmap	= smmu->pgsize_bitmap,
@@ -790,6 +785,12 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 		.iommu_dev	= smmu->dev,
 	};
 
+	if (smmu->impl && smmu->impl->init_context) {
+		ret = smmu->impl->init_context(smmu_domain, &pgtbl_cfg);
+		if (ret)
+			goto out_unlock;
+	}
+
 	if (smmu_domain->non_strict)
 		pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
 
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index afab9de..0eb498f 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -357,7 +357,8 @@ struct arm_smmu_impl {
 			    u64 val);
 	int (*cfg_probe)(struct arm_smmu_device *smmu);
 	int (*reset)(struct arm_smmu_device *smmu);
-	int (*init_context)(struct arm_smmu_domain *smmu_domain);
+	int (*init_context)(struct arm_smmu_domain *smmu_domain,
+			struct io_pgtable_cfg *pgtbl_cfg);
 	void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
 			 int status);
 };
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
                   ` (2 preceding siblings ...)
  2019-11-22 23:31 ` [PATCH v2 3/8] iommu/arm-smmu: Pass io_pgtable_cfg to impl specific init_context Jordan Crouse
@ 2019-11-22 23:31 ` Jordan Crouse
  2019-11-22 23:31 ` [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization Jordan Crouse
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:31 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm,
	linux-kernel, Joerg Roedel

Add implementation specific support to enable split pagetables for
SMMU implementations attached to Adreno GPUs on Qualcomm targets.

To enable split pagetables the driver will set an attribute on the domain.
if conditions are correct, set up the hardware to support equally sized
TTBR0 and TTBR1 regions and programs the domain pagetable to TTBR1 to make
it available for global buffers while allowing the GPU the chance to
switch the TTBR0 at runtime for per-context pagetables.

After programming the context, the value of the domain attribute can be
queried to see if split pagetables were successfully programmed. The
domain geometry will be updated so that the caller can determine the
start of the region to generate correct virtual addresses.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/iommu/arm-smmu-impl.c |  3 ++
 drivers/iommu/arm-smmu-qcom.c | 96 +++++++++++++++++++++++++++++++++++++++++++
 drivers/iommu/arm-smmu.c      | 41 ++++++++++++++----
 drivers/iommu/arm-smmu.h      | 11 +++++
 4 files changed, 143 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index 33ed682..1e91231 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -174,5 +174,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
 	if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
 		return qcom_smmu_impl_init(smmu);
 
+	if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu-v2"))
+		return adreno_smmu_impl_init(smmu);
+
 	return smmu;
 }
diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index 24c071c..6591e49 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -11,6 +11,102 @@ struct qcom_smmu {
 	struct arm_smmu_device smmu;
 };
 
+#define TG0_4K  0
+#define TG0_64K 1
+#define TG0_16K 2
+
+#define TG1_16K 1
+#define TG1_4K  2
+#define TG1_64K 3
+
+/*
+ * Set up split pagetables for Adreno SMMUs that will keep a static TTBR1 for
+ * global buffers and dynamically switch TTBR0 from the GPU for context specific
+ * pagetables.
+ */
+static int adreno_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
+		struct io_pgtable_cfg *pgtbl_cfg)
+{
+	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
+	struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
+	u32 tcr, tg0;
+
+	/*
+	 * Return error if split pagetables are not enabled so that arm-smmu
+	 * do the default configuration
+	 */
+	if (!(pgtbl_cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1))
+		return -EINVAL;
+
+	/* Get the bank configuration from the pagetable config */
+	tcr = arm_smmu_lpae_tcr(pgtbl_cfg) & 0xffff;
+
+	/*
+	 * The TCR configuration for TTBR0 and TTBR1 is (almost) identical so
+	 * just duplicate the T0 configuration and shift it
+	 */
+	cb->tcr[0] = (tcr << 16) | tcr;
+
+	/*
+	 * The (almost) above refers to the granule size field which is
+	 * different for TTBR0 and TTBR1. With the TTBR1 quirk enabled,
+	 * io-pgtable-arm will write the T1 appropriate granule size for tg.
+	 * Translate the configuration from the T1 field to get the right value
+	 * for T0
+	 */
+	if (pgtbl_cfg->arm_lpae_s1_cfg.tcr.tg == TG1_4K)
+		tg0 = TG0_4K;
+	else if (pgtbl_cfg->arm_lpae_s1_cfg.tcr.tg == TG1_16K)
+		tg0 = TG0_16K;
+	else
+		tg0 = TG0_64K;
+
+	/* clear and set the correct value for TG0  */
+	cb->tcr[0] &= ~TCR_TG0;
+	cb->tcr[0] |= FIELD_PREP(TCR_TG0, tg0);
+
+	/*
+	 * arm_smmu_lape_tcr2 sets SEP_UPSTREAM which is always the appropriate
+	 * SEP for Adreno IOMMU
+	 */
+	cb->tcr[1] = arm_smmu_lpae_tcr2(pgtbl_cfg);
+	cb->tcr[1] |= TCR2_AS;
+
+	/* TTBRs */
+	cb->ttbr[0] = FIELD_PREP(TTBRn_ASID, cfg->asid);
+	cb->ttbr[1] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
+	cb->ttbr[1] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
+
+	/* MAIRs */
+	cb->mair[0] = pgtbl_cfg->arm_lpae_s1_cfg.mair;
+	cb->mair[1] = pgtbl_cfg->arm_lpae_s1_cfg.mair >> 32;
+
+	return 0;
+}
+
+static int adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
+		struct io_pgtable_cfg *pgtbl_cfg)
+{
+	/* Enable split pagetables if the flag is set and the format matches */
+	if (smmu_domain->split_pagetables)
+		if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
+			smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64)
+			pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
+
+	return 0;
+}
+
+static const struct arm_smmu_impl adreno_smmu_impl = {
+	.init_context = adreno_smmu_init_context,
+	.init_context_bank = adreno_smmu_init_context_bank,
+};
+
+struct arm_smmu_device *adreno_smmu_impl_init(struct arm_smmu_device *smmu)
+{
+	smmu->impl = &adreno_smmu_impl;
+	return smmu;
+}
+
 static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
 {
 	int ret;
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5c7c32b..f5dc950 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -91,13 +91,6 @@ struct arm_smmu_smr {
 	bool				valid;
 };
 
-struct arm_smmu_cb {
-	u64				ttbr[2];
-	u32				tcr[2];
-	u32				mair[2];
-	struct arm_smmu_cfg		*cfg;
-};
-
 struct arm_smmu_master_cfg {
 	struct arm_smmu_device		*smmu;
 	s16				smendx[];
@@ -512,10 +505,20 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
 {
 	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
 	struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
+	struct arm_smmu_device *smmu = smmu_domain->smmu;
 	bool stage1 = cfg->cbar != CBAR_TYPE_S2_TRANS;
 
 	cb->cfg = cfg;
 
+	if (smmu->impl && smmu->impl->init_context_bank) {
+		/*
+		 * If the implementation specific function returns non-zero then
+		 * fall back to the default configuration
+		 */
+		if (!smmu->impl->init_context_bank(smmu_domain, pgtbl_cfg))
+			return;
+	}
+
 	/* TCR */
 	if (stage1) {
 		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
@@ -802,7 +805,17 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
 
 	/* Update the domain's page sizes to reflect the page table format */
 	domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
-	domain->geometry.aperture_end = (1UL << ias) - 1;
+
+	if (pgtbl_cfg.quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
+		domain->geometry.aperture_start = ~((1UL << ias) - 1);
+		domain->geometry.aperture_end = ~0UL;
+	} else {
+		domain->geometry.aperture_start = 0;
+		domain->geometry.aperture_end = (1UL << ias) - 1;
+
+		smmu_domain->split_pagetables = false;
+	}
+
 	domain->geometry.force_aperture = true;
 
 	/* Initialise the context bank with our page table cfg */
@@ -1485,6 +1498,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
 		case DOMAIN_ATTR_NESTING:
 			*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
 			return 0;
+		case DOMAIN_ATTR_SPLIT_TABLES:
+			*(int *)data = smmu_domain->split_pagetables;
+			return 0;
 		default:
 			return -ENODEV;
 		}
@@ -1525,6 +1541,15 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
 			else
 				smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
 			break;
+		case DOMAIN_ATTR_SPLIT_TABLES:
+			if (smmu_domain->smmu) {
+				return -EPERM;
+				goto out_unlock;
+			}
+
+			if (*(int *) data)
+				smmu_domain->split_pagetables = true;
+			break;
 		default:
 			ret = -ENODEV;
 		}
diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
index 0eb498f..35158ee 100644
--- a/drivers/iommu/arm-smmu.h
+++ b/drivers/iommu/arm-smmu.h
@@ -329,6 +329,14 @@ struct arm_smmu_domain {
 	struct mutex			init_mutex; /* Protects smmu pointer */
 	spinlock_t			cb_lock; /* Serialises ATS1* ops and TLB syncs */
 	struct iommu_domain		domain;
+	bool				split_pagetables;
+};
+
+struct arm_smmu_cb {
+	u64				ttbr[2];
+	u32				tcr[2];
+	u32				mair[2];
+	struct arm_smmu_cfg		*cfg;
 };
 
 static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
@@ -359,6 +367,8 @@ struct arm_smmu_impl {
 	int (*reset)(struct arm_smmu_device *smmu);
 	int (*init_context)(struct arm_smmu_domain *smmu_domain,
 			struct io_pgtable_cfg *pgtbl_cfg);
+	int (*init_context_bank)(struct arm_smmu_domain *smmu_domain,
+			struct io_pgtable_cfg *pgtable_cfg);
 	void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
 			 int status);
 };
@@ -425,6 +435,7 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
 
 struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
 struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
+struct arm_smmu_device *adreno_smmu_impl_init(struct arm_smmu_device *smmu);
 
 int arm_mmu500_reset(struct arm_smmu_device *smmu);
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
                   ` (3 preceding siblings ...)
  2019-11-22 23:31 ` [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations Jordan Crouse
@ 2019-11-22 23:31 ` Jordan Crouse
  2019-11-22 23:32 ` [PATCH v2 8/8] arm64: dts: qcom: sdm845: Update Adreno GPU SMMU compatible string Jordan Crouse
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:31 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm,
	David Airlie, Sean Paul, Bruce Wang, AngeloGioacchino Del Regno,
	Sam Ravnborg, dri-devel, Jeykumar Sankaran, Rob Clark,
	Thomas Gleixner, Jonathan Marek, Drew Davenport, Georgi Djakov,
	freedreno, Jeffrey Hugo, Mamta Shukla, linux-kernel,
	Daniel Vetter

Everywhere an IOMMU object is created by msm_gpu_create_address_space
the IOMMU device is attached immediately after. Instead of carrying around
the infrastructure to do the attach from the device specific code do it
directly in the msm_iommu_init() function. This gets it out of the way for
more aggressive cleanups that follow.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  |  8 --------
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c |  4 ----
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  7 -------
 drivers/gpu/drm/msm/msm_gem_vma.c        | 23 +++++++++++++++++++----
 drivers/gpu/drm/msm/msm_gpu.c            | 11 +----------
 drivers/gpu/drm/msm/msm_gpummu.c         |  6 ------
 drivers/gpu/drm/msm/msm_iommu.c          | 15 +++++++--------
 drivers/gpu/drm/msm/msm_mmu.h            |  1 -
 8 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index 6c92f0f..b082b23 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -704,7 +704,6 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
 {
 	struct iommu_domain *domain;
 	struct msm_gem_address_space *aspace;
-	int ret;
 
 	domain = iommu_domain_alloc(&platform_bus_type);
 	if (!domain)
@@ -720,13 +719,6 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
 		return PTR_ERR(aspace);
 	}
 
-	ret = aspace->mmu->funcs->attach(aspace->mmu);
-	if (ret) {
-		DPU_ERROR("failed to attach iommu %d\n", ret);
-		msm_gem_address_space_put(aspace);
-		return ret;
-	}
-
 	dpu_kms->base.aspace = aspace;
 	return 0;
 }
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index dda0543..9dba37c 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -518,10 +518,6 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
 		}
 
 		kms->aspace = aspace;
-
-		ret = aspace->mmu->funcs->attach(aspace->mmu);
-		if (ret)
-			goto fail;
 	} else {
 		DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys "
 				"contig buffers for scanout\n");
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index e43ecd4..653dab2 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -736,13 +736,6 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
 		}
 
 		kms->aspace = aspace;
-
-		ret = aspace->mmu->funcs->attach(aspace->mmu);
-		if (ret) {
-			DRM_DEV_ERROR(&pdev->dev, "failed to attach iommu: %d\n",
-				ret);
-			goto fail;
-		}
 	} else {
 		DRM_DEV_INFO(&pdev->dev,
 			 "no iommu, fallback to phys contig buffers for scanout\n");
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 1af5354..91d993a 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -131,8 +131,8 @@ msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
 		const char *name)
 {
 	struct msm_gem_address_space *aspace;
-	u64 size = domain->geometry.aperture_end -
-		domain->geometry.aperture_start;
+	u64 start = domain->geometry.aperture_start;
+	u64 size = domain->geometry.aperture_end - start;
 
 	aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
 	if (!aspace)
@@ -141,9 +141,18 @@ msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
 	spin_lock_init(&aspace->lock);
 	aspace->name = name;
 	aspace->mmu = msm_iommu_new(dev, domain);
+	if (IS_ERR(aspace->mmu)) {
+		int ret = PTR_ERR(aspace->mmu);
 
-	drm_mm_init(&aspace->mm, (domain->geometry.aperture_start >> PAGE_SHIFT),
-		size >> PAGE_SHIFT);
+		kfree(aspace);
+		return ERR_PTR(ret);
+	}
+
+	/*
+	 * Attaching the IOMMU device changes the aperture values so use the
+	 * cached values instead
+	 */
+	drm_mm_init(&aspace->mm, start >> PAGE_SHIFT, size >> PAGE_SHIFT);
 
 	kref_init(&aspace->kref);
 
@@ -164,6 +173,12 @@ msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
 	spin_lock_init(&aspace->lock);
 	aspace->name = name;
 	aspace->mmu = msm_gpummu_new(dev, gpu);
+	if (IS_ERR(aspace->mmu)) {
+		int ret = PTR_ERR(aspace->mmu);
+
+		kfree(aspace);
+		return ERR_PTR(ret);
+	}
 
 	drm_mm_init(&aspace->mm, (va_start >> PAGE_SHIFT),
 		size >> PAGE_SHIFT);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 18f3a5c..f7bf80e 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -808,7 +808,6 @@ msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
 		uint64_t va_start, uint64_t va_end)
 {
 	struct msm_gem_address_space *aspace;
-	int ret;
 
 	/*
 	 * Setup IOMMU.. eventually we will (I think) do this once per context
@@ -833,17 +832,9 @@ msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
 			va_start, va_end);
 	}
 
-	if (IS_ERR(aspace)) {
+	if (IS_ERR(aspace))
 		DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
 			PTR_ERR(aspace));
-		return ERR_CAST(aspace);
-	}
-
-	ret = aspace->mmu->funcs->attach(aspace->mmu);
-	if (ret) {
-		msm_gem_address_space_put(aspace);
-		return ERR_PTR(ret);
-	}
 
 	return aspace;
 }
diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c
index 34980d8..0ad0f84 100644
--- a/drivers/gpu/drm/msm/msm_gpummu.c
+++ b/drivers/gpu/drm/msm/msm_gpummu.c
@@ -21,11 +21,6 @@ struct msm_gpummu {
 #define GPUMMU_PAGE_SIZE SZ_4K
 #define TABLE_SIZE (sizeof(uint32_t) * GPUMMU_VA_RANGE / GPUMMU_PAGE_SIZE)
 
-static int msm_gpummu_attach(struct msm_mmu *mmu)
-{
-	return 0;
-}
-
 static void msm_gpummu_detach(struct msm_mmu *mmu)
 {
 }
@@ -85,7 +80,6 @@ static void msm_gpummu_destroy(struct msm_mmu *mmu)
 }
 
 static const struct msm_mmu_funcs funcs = {
-		.attach = msm_gpummu_attach,
 		.detach = msm_gpummu_detach,
 		.map = msm_gpummu_map,
 		.unmap = msm_gpummu_unmap,
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index ad58cfe..544c519 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -23,13 +23,6 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
 	return 0;
 }
 
-static int msm_iommu_attach(struct msm_mmu *mmu)
-{
-	struct msm_iommu *iommu = to_msm_iommu(mmu);
-
-	return iommu_attach_device(iommu->domain, mmu->dev);
-}
-
 static void msm_iommu_detach(struct msm_mmu *mmu)
 {
 	struct msm_iommu *iommu = to_msm_iommu(mmu);
@@ -66,7 +59,6 @@ static void msm_iommu_destroy(struct msm_mmu *mmu)
 }
 
 static const struct msm_mmu_funcs funcs = {
-		.attach = msm_iommu_attach,
 		.detach = msm_iommu_detach,
 		.map = msm_iommu_map,
 		.unmap = msm_iommu_unmap,
@@ -76,6 +68,7 @@ static const struct msm_mmu_funcs funcs = {
 struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
 {
 	struct msm_iommu *iommu;
+	int ret;
 
 	iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
 	if (!iommu)
@@ -85,5 +78,11 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
 	msm_mmu_init(&iommu->base, dev, &funcs);
 	iommu_set_fault_handler(domain, msm_fault_handler, iommu);
 
+	ret = iommu_attach_device(iommu->domain, dev);
+	if (ret) {
+		kfree(iommu);
+		return ERR_PTR(ret);
+	}
+
 	return &iommu->base;
 }
diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
index 67a623f..bae9e8e 100644
--- a/drivers/gpu/drm/msm/msm_mmu.h
+++ b/drivers/gpu/drm/msm/msm_mmu.h
@@ -10,7 +10,6 @@
 #include <linux/iommu.h>
 
 struct msm_mmu_funcs {
-	int (*attach)(struct msm_mmu *mmu);
 	void (*detach)(struct msm_mmu *mmu);
 	int (*map)(struct msm_mmu *mmu, uint64_t iova, struct sg_table *sgt,
 			unsigned len, int prot);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 8/8] arm64: dts: qcom: sdm845:  Update Adreno GPU SMMU compatible string
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
                   ` (4 preceding siblings ...)
  2019-11-22 23:31 ` [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization Jordan Crouse
@ 2019-11-22 23:32 ` Jordan Crouse
  2019-11-22 23:32 ` [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables Jordan Crouse
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:32 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm, devicetree,
	Bjorn Andersson, linux-kernel, Andy Gross, Rob Herring,
	Mark Rutland

Add "qcom,adreno-smmu-v2" compatible string for the Adreno GPU SMMU node
to enable split pagetable support.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index ddb1f23..d90ba6eda 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2869,7 +2869,7 @@
 		};
 
 		adreno_smmu: iommu@5040000 {
-			compatible = "qcom,sdm845-smmu-v2", "qcom,smmu-v2";
+			compatible = "qcom,adreno-smmu-v2", "qcom,smmu-v2";
 			reg = <0 0x5040000 0 0x10000>;
 			#iommu-cells = <1>;
 			#global-interrupts = <2>;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
                   ` (5 preceding siblings ...)
  2019-11-22 23:32 ` [PATCH v2 8/8] arm64: dts: qcom: sdm845: Update Adreno GPU SMMU compatible string Jordan Crouse
@ 2019-11-22 23:32 ` Jordan Crouse
  2019-11-22 23:32 ` [PATCH v2 6/8] drm/msm: Refactor address space initialization Jordan Crouse
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:32 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm, Sean Paul,
	linux-kernel, dri-devel, Rob Clark, David Airlie, freedreno,
	Mamta Shukla, Daniel Vetter

Attempt to enable split pagetables if the arm-smmu driver supports it.
This will move the default address space from the default region to
the address range assigned to TTBR1. The behavior should be transparent
to the driver for now but it gets the default buffers out of the way
when we want to start swapping TTBR0 for context-specific pagetables.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 ++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 5dc0b2c..96b3b28 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -811,6 +811,50 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
 	return (unsigned long)busy_time;
 }
 
+static struct msm_gem_address_space *
+a6xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev)
+{
+	struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
+	struct msm_gem_address_space *aspace;
+	struct msm_mmu *mmu;
+	u64 start, size;
+	u32 val = 1;
+	int ret;
+
+	if (!iommu)
+		return ERR_PTR(-ENOMEM);
+
+	/* Try to request split pagetables */
+	iommu_domain_set_attr(iommu, DOMAIN_ATTR_SPLIT_TABLES, &val);
+
+	mmu = msm_iommu_new(&pdev->dev, iommu);
+	if (IS_ERR(mmu)) {
+		iommu_domain_free(iommu);
+		return ERR_CAST(mmu);
+	}
+
+	/* Check to see if split pagetables were successful */
+	ret = iommu_domain_get_attr(iommu, DOMAIN_ATTR_SPLIT_TABLES, &val);
+	if (!ret && val) {
+		/*
+		 * The aperture start will be at the beginning of the TTBR1
+		 * space so use that as a base
+		 */
+		start = iommu->geometry.aperture_start;
+		size = 0xffffffff;
+	} else {
+		/* Otherwise use the legacy 32 bit region */
+		start = SZ_16M;
+		size = 0xffffffff - SZ_16M;
+	}
+
+	aspace = msm_gem_address_space_create(mmu, "gpu", start, size);
+	if (IS_ERR(aspace))
+		iommu_domain_free(iommu);
+
+	return aspace;
+}
+
 static const struct adreno_gpu_funcs funcs = {
 	.base = {
 		.get_param = adreno_get_param,
@@ -832,7 +876,7 @@ static const struct adreno_gpu_funcs funcs = {
 #if defined(CONFIG_DRM_MSM_GPU_STATE)
 		.gpu_state_get = a6xx_gpu_state_get,
 		.gpu_state_put = a6xx_gpu_state_put,
-		.create_address_space = adreno_iommu_create_address_space,
+		.create_address_space = a6xx_create_address_space,
 #endif
 	},
 	.get_timestamp = a6xx_get_timestamp,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/8] drm/msm: Refactor address space initialization
       [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
                   ` (6 preceding siblings ...)
  2019-11-22 23:32 ` [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables Jordan Crouse
@ 2019-11-22 23:32 ` Jordan Crouse
       [not found] ` <0101016e95751c0b-33c9379b-6b8c-43b1-8785-e5e1b6f084f1-000000@us-west-2.amazonses.com>
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-11-22 23:32 UTC (permalink / raw)
  To: iommu
  Cc: robin.murphy, will, linux-arm-kernel, linux-arm-msm, Sean Paul,
	Sharat Masetty, dri-devel, linux-kernel, David Airlie,
	Allison Randal, Ben Dooks, Thomas Gleixner, Douglas Anderson,
	Rob Clark, Greg Kroah-Hartman, Jeffrey Hugo, Wen Yang,
	Alexios Zavras, Jeykumar Sankaran, AngeloGioacchino Del Regno,
	Bruce Wang, Sam Ravnborg, Brian Masney, Jonathan Marek,
	Drew Davenport, Georgi Djakov, freedreno, Mamta Shukla,
	Daniel Vetter

Refactor how address space initialization works. Instead of having the
address space function create the MMU object (and thus require separate but
equal functions for gpummu and iommu) use a single function and pass the
MMU struct in. Make the generic code cleaner by using target specific
functions to create the address space so a2xx can do its own thing in its
own space.  For all the other targets use a generic helper to initialize
IOMMU but leave the door open for newer targets to use customization
if they need it.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a2xx_gpu.c    | 16 ++++++++++
 drivers/gpu/drm/msm/adreno/a3xx_gpu.c    |  1 +
 drivers/gpu/drm/msm/adreno/a4xx_gpu.c    |  1 +
 drivers/gpu/drm/msm/adreno/a5xx_gpu.c    |  1 +
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c    |  1 +
 drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 23 ++++++++++----
 drivers/gpu/drm/msm/adreno/adreno_gpu.h  |  8 +++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 10 +++---
 drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 14 +++++----
 drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c |  4 ---
 drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 11 +++++--
 drivers/gpu/drm/msm/msm_drv.h            |  8 ++---
 drivers/gpu/drm/msm/msm_gem_vma.c        | 52 +++++---------------------------
 drivers/gpu/drm/msm/msm_gpu.c            | 40 ++----------------------
 drivers/gpu/drm/msm/msm_gpu.h            |  4 +--
 drivers/gpu/drm/msm/msm_iommu.c          |  3 ++
 16 files changed, 83 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
index 1f83bc1..60f6472 100644
--- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
@@ -401,6 +401,21 @@ static struct msm_gpu_state *a2xx_gpu_state_get(struct msm_gpu *gpu)
 	return state;
 }
 
+static struct msm_gem_address_space *
+a2xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev)
+{
+	struct msm_mmu *mmu = msm_gpummu_new(&pdev->dev, gpu);
+	struct msm_gem_address_space *aspace;
+
+	aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
+		SZ_16M + 0xfff * SZ_64K);
+
+	if (IS_ERR(aspace) && !IS_ERR(mmu))
+		mmu->funcs->destroy(mmu);
+
+	return aspace;
+}
+
 /* Register offset defines for A2XX - copy of A3XX */
 static const unsigned int a2xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
 	REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
@@ -429,6 +444,7 @@ static const struct adreno_gpu_funcs funcs = {
 #endif
 		.gpu_state_get = a2xx_gpu_state_get,
 		.gpu_state_put = adreno_gpu_state_put,
+		.create_address_space = a2xx_create_address_space,
 	},
 };
 
diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
index 7ad1493..41e51e0 100644
--- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
@@ -441,6 +441,7 @@ static const struct adreno_gpu_funcs funcs = {
 #endif
 		.gpu_state_get = a3xx_gpu_state_get,
 		.gpu_state_put = adreno_gpu_state_put,
+		.create_address_space = adreno_iommu_create_address_space,
 	},
 };
 
diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
index b01388a..3655440 100644
--- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
@@ -532,6 +532,7 @@ static const struct adreno_gpu_funcs funcs = {
 #endif
 		.gpu_state_get = a4xx_gpu_state_get,
 		.gpu_state_put = adreno_gpu_state_put,
+		.create_address_space = adreno_iommu_create_address_space,
 	},
 	.get_timestamp = a4xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index b02e204..0f5db72 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1432,6 +1432,7 @@ static const struct adreno_gpu_funcs funcs = {
 		.gpu_busy = a5xx_gpu_busy,
 		.gpu_state_get = a5xx_gpu_state_get,
 		.gpu_state_put = a5xx_gpu_state_put,
+		.create_address_space = adreno_iommu_create_address_space,
 	},
 	.get_timestamp = a5xx_get_timestamp,
 };
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index dc8ec2c..5dc0b2c 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -832,6 +832,7 @@ static const struct adreno_gpu_funcs funcs = {
 #if defined(CONFIG_DRM_MSM_GPU_STATE)
 		.gpu_state_get = a6xx_gpu_state_get,
 		.gpu_state_put = a6xx_gpu_state_put,
+		.create_address_space = adreno_iommu_create_address_space,
 #endif
 	},
 	.get_timestamp = a6xx_get_timestamp,
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 0783e4b..09c57891 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -157,6 +157,23 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
 	return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, pasid);
 }
 
+struct msm_gem_address_space *
+adreno_iommu_create_address_space(struct msm_gpu *gpu,
+		struct platform_device *pdev)
+{
+	struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
+	struct msm_mmu *mmu = msm_iommu_new(&pdev->dev, iommu);
+	struct msm_gem_address_space *aspace;
+
+	aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
+		0xfffffff);
+
+	if (IS_ERR(aspace) && !IS_ERR(mmu))
+		mmu->funcs->destroy(mmu);
+
+	return aspace;
+}
+
 int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
 {
 	struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -949,12 +966,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 	adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
 
-	adreno_gpu_config.va_start = SZ_16M;
-	adreno_gpu_config.va_end = 0xffffffff;
-	/* maximum range of a2xx mmu */
-	if (adreno_is_a2xx(adreno_gpu))
-		adreno_gpu_config.va_end = SZ_16M + 0xfff * SZ_64K;
-
 	adreno_gpu_config.nr_rings = nr_rings;
 
 	adreno_get_pwrlevels(&pdev->dev, gpu);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index e71a757..5c1aa12 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -263,6 +263,14 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
 int adreno_gpu_state_put(struct msm_gpu_state *state);
 
 /*
+ * Common helper function to initialize the default address space for arm-smmu
+ * attached targets
+ */
+struct msm_gem_address_space *
+adreno_iommu_create_address_space(struct msm_gpu *gpu,
+		struct platform_device *pdev);
+
+/*
  * For a5xx and a6xx targets load the zap shader that is used to pull the GPU
  * out of secure mode
  */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index b082b23..4e6ebbd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -704,18 +704,18 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
 {
 	struct iommu_domain *domain;
 	struct msm_gem_address_space *aspace;
+	struct msm_mmu *mmu;
 
 	domain = iommu_domain_alloc(&platform_bus_type);
 	if (!domain)
 		return 0;
 
-	domain->geometry.aperture_start = 0x1000;
-	domain->geometry.aperture_end = 0xffffffff;
+	mmu = msm_iommu_new(dpu_kms->dev->dev, domain);
+	aspace = msm_gem_address_space_create(mmu, "dpu1",
+		0x1000, 0xfffffff);
 
-	aspace = msm_gem_address_space_create(dpu_kms->dev->dev,
-			domain, "dpu1");
 	if (IS_ERR(aspace)) {
-		iommu_domain_free(domain);
+		mmu->funcs->destroy(mmu);
 		return PTR_ERR(aspace);
 	}
 
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
index 9dba37c..0889718 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
@@ -510,9 +510,15 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
 	mdelay(16);
 
 	if (config->iommu) {
-		aspace = msm_gem_address_space_create(&pdev->dev,
-				config->iommu, "mdp4");
+		struct msm_mmu *mmu = msm_iommu_new(&pdev->dev,
+			config->iommu);
+
+		aspace  = msm_gem_address_space_create(mmu,
+			"mdp4", 0x1000, 0xffffffff);
+
 		if (IS_ERR(aspace)) {
+			if (!IS_ERR(mmu))
+				mmu->funcs->destroy(mmu);
 			ret = PTR_ERR(aspace);
 			goto fail;
 		}
@@ -565,10 +571,6 @@ static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev)
 	/* TODO: Chips that aren't apq8064 have a 200 Mhz max_clk */
 	config.max_clk = 266667000;
 	config.iommu = iommu_domain_alloc(&platform_bus_type);
-	if (config.iommu) {
-		config.iommu->geometry.aperture_start = 0x1000;
-		config.iommu->geometry.aperture_end = 0xffffffff;
-	}
 
 	return &config;
 }
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
index 1f48f64..ebd651a 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
@@ -941,10 +941,6 @@ static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
 	static struct mdp5_cfg_platform config = {};
 
 	config.iommu = iommu_domain_alloc(&platform_bus_type);
-	if (config.iommu) {
-		config.iommu->geometry.aperture_start = 0x1000;
-		config.iommu->geometry.aperture_end = 0xffffffff;
-	}
 
 	return &config;
 }
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 653dab2..20bdff9 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -724,13 +724,20 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
 	mdelay(16);
 
 	if (config->platform.iommu) {
+		struct msm_mmu *mmu;
+
 		iommu_dev = &pdev->dev;
 		if (!iommu_dev->iommu_fwspec)
 			iommu_dev = iommu_dev->parent;
 
-		aspace = msm_gem_address_space_create(iommu_dev,
-				config->platform.iommu, "mdp5");
+		mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
+
+		aspace = msm_gem_address_space_create(mmu, "mdp5",
+			0x1000, 0xffffffff);
+
 		if (IS_ERR(aspace)) {
+			if (!IS_ERR(mmu))
+				mmu->funcs->destroy(mmu);
 			ret = PTR_ERR(aspace);
 			goto fail;
 		}
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 71547e7..2203729 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -247,12 +247,8 @@ void msm_gem_close_vma(struct msm_gem_address_space *aspace,
 void msm_gem_address_space_put(struct msm_gem_address_space *aspace);
 
 struct msm_gem_address_space *
-msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
-		const char *name);
-
-struct msm_gem_address_space *
-msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
-		const char *name, uint64_t va_start, uint64_t va_end);
+msm_gem_address_space_create(struct msm_mmu *mmu, const char *name,
+		u64 va_start, u64 va_end);
 
 int msm_register_mmu(struct drm_device *dev, struct msm_mmu *mmu);
 void msm_unregister_mmu(struct drm_device *dev, struct msm_mmu *mmu);
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
index 91d993a..075ce52 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -125,63 +125,25 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace,
 	return 0;
 }
 
-
 struct msm_gem_address_space *
-msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
-		const char *name)
-{
-	struct msm_gem_address_space *aspace;
-	u64 start = domain->geometry.aperture_start;
-	u64 size = domain->geometry.aperture_end - start;
-
-	aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
-	if (!aspace)
-		return ERR_PTR(-ENOMEM);
-
-	spin_lock_init(&aspace->lock);
-	aspace->name = name;
-	aspace->mmu = msm_iommu_new(dev, domain);
-	if (IS_ERR(aspace->mmu)) {
-		int ret = PTR_ERR(aspace->mmu);
-
-		kfree(aspace);
-		return ERR_PTR(ret);
-	}
-
-	/*
-	 * Attaching the IOMMU device changes the aperture values so use the
-	 * cached values instead
-	 */
-	drm_mm_init(&aspace->mm, start >> PAGE_SHIFT, size >> PAGE_SHIFT);
-
-	kref_init(&aspace->kref);
-
-	return aspace;
-}
-
-struct msm_gem_address_space *
-msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
-		const char *name, uint64_t va_start, uint64_t va_end)
+msm_gem_address_space_create(struct msm_mmu *mmu, const char *name,
+		u64 va_start, u64 va_end)
 {
 	struct msm_gem_address_space *aspace;
 	u64 size = va_end - va_start;
 
+	if (IS_ERR(mmu))
+		return ERR_CAST(mmu);
+
 	aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
 	if (!aspace)
 		return ERR_PTR(-ENOMEM);
 
 	spin_lock_init(&aspace->lock);
 	aspace->name = name;
-	aspace->mmu = msm_gpummu_new(dev, gpu);
-	if (IS_ERR(aspace->mmu)) {
-		int ret = PTR_ERR(aspace->mmu);
-
-		kfree(aspace);
-		return ERR_PTR(ret);
-	}
+	aspace->mmu = mmu;
 
-	drm_mm_init(&aspace->mm, (va_start >> PAGE_SHIFT),
-		size >> PAGE_SHIFT);
+	drm_mm_init(&aspace->mm, va_start >> PAGE_SHIFT, size >> PAGE_SHIFT);
 
 	kref_init(&aspace->kref);
 
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index f7bf80e..f11df53 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -803,42 +803,6 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu)
 	return 0;
 }
 
-static struct msm_gem_address_space *
-msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
-		uint64_t va_start, uint64_t va_end)
-{
-	struct msm_gem_address_space *aspace;
-
-	/*
-	 * Setup IOMMU.. eventually we will (I think) do this once per context
-	 * and have separate page tables per context.  For now, to keep things
-	 * simple and to get something working, just use a single address space:
-	 */
-	if (!adreno_is_a2xx(to_adreno_gpu(gpu))) {
-		struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
-		if (!iommu)
-			return NULL;
-
-		iommu->geometry.aperture_start = va_start;
-		iommu->geometry.aperture_end = va_end;
-
-		DRM_DEV_INFO(gpu->dev->dev, "%s: using IOMMU\n", gpu->name);
-
-		aspace = msm_gem_address_space_create(&pdev->dev, iommu, "gpu");
-		if (IS_ERR(aspace))
-			iommu_domain_free(iommu);
-	} else {
-		aspace = msm_gem_address_space_create_a2xx(&pdev->dev, gpu, "gpu",
-			va_start, va_end);
-	}
-
-	if (IS_ERR(aspace))
-		DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
-			PTR_ERR(aspace));
-
-	return aspace;
-}
-
 int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 		struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
 		const char *name, struct msm_gpu_config *config)
@@ -911,8 +875,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
 
 	msm_devfreq_init(gpu);
 
-	gpu->aspace = msm_gpu_create_address_space(gpu, pdev,
-		config->va_start, config->va_end);
+
+	gpu->aspace = gpu->funcs->create_address_space(gpu, pdev);
 
 	if (gpu->aspace == NULL)
 		DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index ab8f0f9c..41d86c2 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -21,8 +21,6 @@ struct msm_gpu_state;
 
 struct msm_gpu_config {
 	const char *ioname;
-	uint64_t va_start;
-	uint64_t va_end;
 	unsigned int nr_rings;
 };
 
@@ -64,6 +62,8 @@ struct msm_gpu_funcs {
 	int (*gpu_state_put)(struct msm_gpu_state *state);
 	unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
 	void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq);
+	struct msm_gem_address_space *(*create_address_space)
+		(struct msm_gpu *gpu, struct platform_device *pdev);
 };
 
 struct msm_gpu {
diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
index 544c519..e773ef8 100644
--- a/drivers/gpu/drm/msm/msm_iommu.c
+++ b/drivers/gpu/drm/msm/msm_iommu.c
@@ -70,6 +70,9 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
 	struct msm_iommu *iommu;
 	int ret;
 
+	if (!domain)
+		return ERR_PTR(-ENODEV);
+
 	iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
 	if (!iommu)
 		return ERR_PTR(-ENOMEM);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant
  2019-11-22 23:31 ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Jordan Crouse
@ 2019-12-03 19:14   ` Rob Herring
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Herring @ 2019-12-03 19:14 UTC (permalink / raw)
  To: Jordan Crouse
  Cc: iommu, robin.murphy, will, linux-arm-kernel, linux-arm-msm,
	devicetree, linux-kernel, Mark Rutland, Joerg Roedel

On Fri, Nov 22, 2019 at 11:31:51PM +0000, Jordan Crouse wrote:
> Add a compatible string to identify SMMUs that are attached
> to Adreno GPU devices that wish to support split pagetables.
> 
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> 
>  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant
       [not found] ` <0101016e95751c0b-33c9379b-6b8c-43b1-8785-e5e1b6f084f1-000000@us-west-2.amazonses.com>
@ 2019-12-04 15:55   ` Robin Murphy
  2019-12-04 18:01     ` Rob Clark
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2019-12-04 15:55 UTC (permalink / raw)
  To: Jordan Crouse, iommu
  Cc: will, linux-arm-kernel, linux-arm-msm, devicetree, linux-kernel,
	Rob Herring, Mark Rutland, Joerg Roedel

On 22/11/2019 11:31 pm, Jordan Crouse wrote:
> Add a compatible string to identify SMMUs that are attached
> to Adreno GPU devices that wish to support split pagetables.

A software policy decision is not, in itself, a good justification for a 
DT property. Is the GPU SMMU fundamentally different in hardware* from 
the other SMMU(s) in any given SoC?

(* where "hardware" may encompass hypervisor shenanigans)

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> 
>   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> index 6515dbe..db9f826 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> @@ -31,6 +31,12 @@ properties:
>                 - qcom,sdm845-smmu-v2
>             - const: qcom,smmu-v2
>   
> +      - description: Qcom Adreno GPU SMMU iplementing split pagetables
> +        items:
> +          - enum:
> +              - qcom,adreno-smmu-v2
> +          - const: qcom,smmu-v2

Given that we already have per-SoC compatibles for Qcom SMMUs in 
general, this seems suspiciously vague.

Robin.

> +
>         - description: Qcom SoCs implementing "arm,mmu-500"
>           items:
>             - enum:
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations
       [not found] ` <0101016e95752703-78491f46-41db-441c-b0fb-9a760e4d56cb-000000@us-west-2.amazonses.com>
@ 2019-12-04 16:44   ` Robin Murphy
  2019-12-05 15:51     ` Jordan Crouse
  0 siblings, 1 reply; 16+ messages in thread
From: Robin Murphy @ 2019-12-04 16:44 UTC (permalink / raw)
  To: Jordan Crouse, iommu
  Cc: will, linux-arm-kernel, linux-arm-msm, linux-kernel, Joerg Roedel

On 22/11/2019 11:31 pm, Jordan Crouse wrote:
> Add implementation specific support to enable split pagetables for
> SMMU implementations attached to Adreno GPUs on Qualcomm targets.
> 
> To enable split pagetables the driver will set an attribute on the domain.
> if conditions are correct, set up the hardware to support equally sized
> TTBR0 and TTBR1 regions and programs the domain pagetable to TTBR1 to make
> it available for global buffers while allowing the GPU the chance to
> switch the TTBR0 at runtime for per-context pagetables.
> 
> After programming the context, the value of the domain attribute can be
> queried to see if split pagetables were successfully programmed. The
> domain geometry will be updated so that the caller can determine the
> start of the region to generate correct virtual addresses.

Why is any of this in impl? It all looks like perfectly generic 
architectural TTBR1 setup to me. As long as DOMAIN_ATTR_SPLIT_TABLES is 
explicitly an opt-in for callers, I'm OK with them having to trust that 
SEP_UPSTREAM is good enough. Or, even better, make the value of 
DOMAIN_ATTR_SPLIT_TABLES not a boolean but the actual split point, where 
the default of 0 would logically mean "no split".

> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
> 
>   drivers/iommu/arm-smmu-impl.c |  3 ++
>   drivers/iommu/arm-smmu-qcom.c | 96 +++++++++++++++++++++++++++++++++++++++++++
>   drivers/iommu/arm-smmu.c      | 41 ++++++++++++++----
>   drivers/iommu/arm-smmu.h      | 11 +++++
>   4 files changed, 143 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index 33ed682..1e91231 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -174,5 +174,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
>   	if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
>   		return qcom_smmu_impl_init(smmu);
>   
> +	if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu-v2"))
> +		return adreno_smmu_impl_init(smmu);
> +
>   	return smmu;
>   }
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> index 24c071c..6591e49 100644
> --- a/drivers/iommu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -11,6 +11,102 @@ struct qcom_smmu {
>   	struct arm_smmu_device smmu;
>   };
>   
> +#define TG0_4K  0
> +#define TG0_64K 1
> +#define TG0_16K 2
> +
> +#define TG1_16K 1
> +#define TG1_4K  2
> +#define TG1_64K 3
> +
> +/*
> + * Set up split pagetables for Adreno SMMUs that will keep a static TTBR1 for
> + * global buffers and dynamically switch TTBR0 from the GPU for context specific
> + * pagetables.
> + */
> +static int adreno_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
> +		struct io_pgtable_cfg *pgtbl_cfg)
> +{
> +	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
> +	struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> +	u32 tcr, tg0;
> +
> +	/*
> +	 * Return error if split pagetables are not enabled so that arm-smmu
> +	 * do the default configuration
> +	 */
> +	if (!(pgtbl_cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1))
> +		return -EINVAL;
> +
> +	/* Get the bank configuration from the pagetable config */
> +	tcr = arm_smmu_lpae_tcr(pgtbl_cfg) & 0xffff;

The intent is that arm_smmu_lpae_tcr() should inherently return the 
appropriate half of the TCR based on pgtable_cfg. It seems like a lot of 
this complexity stems from missing that; sorry if it was unclear.

Robin.

> +
> +	/*
> +	 * The TCR configuration for TTBR0 and TTBR1 is (almost) identical so
> +	 * just duplicate the T0 configuration and shift it
> +	 */
> +	cb->tcr[0] = (tcr << 16) | tcr;
> +
> +	/*
> +	 * The (almost) above refers to the granule size field which is
> +	 * different for TTBR0 and TTBR1. With the TTBR1 quirk enabled,
> +	 * io-pgtable-arm will write the T1 appropriate granule size for tg.
> +	 * Translate the configuration from the T1 field to get the right value
> +	 * for T0
> +	 */
> +	if (pgtbl_cfg->arm_lpae_s1_cfg.tcr.tg == TG1_4K)
> +		tg0 = TG0_4K;
> +	else if (pgtbl_cfg->arm_lpae_s1_cfg.tcr.tg == TG1_16K)
> +		tg0 = TG0_16K;
> +	else
> +		tg0 = TG0_64K;
> +
> +	/* clear and set the correct value for TG0  */
> +	cb->tcr[0] &= ~TCR_TG0;
> +	cb->tcr[0] |= FIELD_PREP(TCR_TG0, tg0);
> +
> +	/*
> +	 * arm_smmu_lape_tcr2 sets SEP_UPSTREAM which is always the appropriate
> +	 * SEP for Adreno IOMMU
> +	 */
> +	cb->tcr[1] = arm_smmu_lpae_tcr2(pgtbl_cfg);
> +	cb->tcr[1] |= TCR2_AS;
> +
> +	/* TTBRs */
> +	cb->ttbr[0] = FIELD_PREP(TTBRn_ASID, cfg->asid);
> +	cb->ttbr[1] = pgtbl_cfg->arm_lpae_s1_cfg.ttbr;
> +	cb->ttbr[1] |= FIELD_PREP(TTBRn_ASID, cfg->asid);
> +
> +	/* MAIRs */
> +	cb->mair[0] = pgtbl_cfg->arm_lpae_s1_cfg.mair;
> +	cb->mair[1] = pgtbl_cfg->arm_lpae_s1_cfg.mair >> 32;
> +
> +	return 0;
> +}
> +
> +static int adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> +		struct io_pgtable_cfg *pgtbl_cfg)
> +{
> +	/* Enable split pagetables if the flag is set and the format matches */
> +	if (smmu_domain->split_pagetables)
> +		if (smmu_domain->stage == ARM_SMMU_DOMAIN_S1 &&
> +			smmu_domain->cfg.fmt == ARM_SMMU_CTX_FMT_AARCH64)
> +			pgtbl_cfg->quirks |= IO_PGTABLE_QUIRK_ARM_TTBR1;
> +
> +	return 0;
> +}
> +
> +static const struct arm_smmu_impl adreno_smmu_impl = {
> +	.init_context = adreno_smmu_init_context,
> +	.init_context_bank = adreno_smmu_init_context_bank,
> +};
> +
> +struct arm_smmu_device *adreno_smmu_impl_init(struct arm_smmu_device *smmu)
> +{
> +	smmu->impl = &adreno_smmu_impl;
> +	return smmu;
> +}
> +
>   static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
>   {
>   	int ret;
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 5c7c32b..f5dc950 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -91,13 +91,6 @@ struct arm_smmu_smr {
>   	bool				valid;
>   };
>   
> -struct arm_smmu_cb {
> -	u64				ttbr[2];
> -	u32				tcr[2];
> -	u32				mair[2];
> -	struct arm_smmu_cfg		*cfg;
> -};
> -
>   struct arm_smmu_master_cfg {
>   	struct arm_smmu_device		*smmu;
>   	s16				smendx[];
> @@ -512,10 +505,20 @@ static void arm_smmu_init_context_bank(struct arm_smmu_domain *smmu_domain,
>   {
>   	struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
>   	struct arm_smmu_cb *cb = &smmu_domain->smmu->cbs[cfg->cbndx];
> +	struct arm_smmu_device *smmu = smmu_domain->smmu;
>   	bool stage1 = cfg->cbar != CBAR_TYPE_S2_TRANS;
>   
>   	cb->cfg = cfg;
>   
> +	if (smmu->impl && smmu->impl->init_context_bank) {
> +		/*
> +		 * If the implementation specific function returns non-zero then
> +		 * fall back to the default configuration
> +		 */
> +		if (!smmu->impl->init_context_bank(smmu_domain, pgtbl_cfg))
> +			return; > +	}
> +
>   	/* TCR */
>   	if (stage1) {
>   		if (cfg->fmt == ARM_SMMU_CTX_FMT_AARCH32_S) {
> @@ -802,7 +805,17 @@ static int arm_smmu_init_domain_context(struct iommu_domain *domain,
>   
>   	/* Update the domain's page sizes to reflect the page table format */
>   	domain->pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
> -	domain->geometry.aperture_end = (1UL << ias) - 1;
> +
> +	if (pgtbl_cfg.quirks & IO_PGTABLE_QUIRK_ARM_TTBR1) {
> +		domain->geometry.aperture_start = ~((1UL << ias) - 1);
> +		domain->geometry.aperture_end = ~0UL;
> +	} else {
> +		domain->geometry.aperture_start = 0;
> +		domain->geometry.aperture_end = (1UL << ias) - 1;
> +
> +		smmu_domain->split_pagetables = false;
> +	}
> +
>   	domain->geometry.force_aperture = true;
>   
>   	/* Initialise the context bank with our page table cfg */
> @@ -1485,6 +1498,9 @@ static int arm_smmu_domain_get_attr(struct iommu_domain *domain,
>   		case DOMAIN_ATTR_NESTING:
>   			*(int *)data = (smmu_domain->stage == ARM_SMMU_DOMAIN_NESTED);
>   			return 0;
> +		case DOMAIN_ATTR_SPLIT_TABLES:
> +			*(int *)data = smmu_domain->split_pagetables;
> +			return 0;
>   		default:
>   			return -ENODEV;
>   		}
> @@ -1525,6 +1541,15 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
>   			else
>   				smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
>   			break;
> +		case DOMAIN_ATTR_SPLIT_TABLES:
> +			if (smmu_domain->smmu) {
> +				return -EPERM;
> +				goto out_unlock;
> +			}
> +
> +			if (*(int *) data)
> +				smmu_domain->split_pagetables = true;
> +			break;
>   		default:
>   			ret = -ENODEV;
>   		}
> diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h
> index 0eb498f..35158ee 100644
> --- a/drivers/iommu/arm-smmu.h
> +++ b/drivers/iommu/arm-smmu.h
> @@ -329,6 +329,14 @@ struct arm_smmu_domain {
>   	struct mutex			init_mutex; /* Protects smmu pointer */
>   	spinlock_t			cb_lock; /* Serialises ATS1* ops and TLB syncs */
>   	struct iommu_domain		domain;
> +	bool				split_pagetables;
> +};
> +
> +struct arm_smmu_cb {
> +	u64				ttbr[2];
> +	u32				tcr[2];
> +	u32				mair[2];
> +	struct arm_smmu_cfg		*cfg;
>   };
>   
>   static inline u32 arm_smmu_lpae_tcr(struct io_pgtable_cfg *cfg)
> @@ -359,6 +367,8 @@ struct arm_smmu_impl {
>   	int (*reset)(struct arm_smmu_device *smmu);
>   	int (*init_context)(struct arm_smmu_domain *smmu_domain,
>   			struct io_pgtable_cfg *pgtbl_cfg);
> +	int (*init_context_bank)(struct arm_smmu_domain *smmu_domain,
> +			struct io_pgtable_cfg *pgtable_cfg);
>   	void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
>   			 int status);
>   };
> @@ -425,6 +435,7 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
>   
>   struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
>   struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
> +struct arm_smmu_device *adreno_smmu_impl_init(struct arm_smmu_device *smmu);
>   
>   int arm_mmu500_reset(struct arm_smmu_device *smmu);
>   
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant
  2019-12-04 15:55   ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Robin Murphy
@ 2019-12-04 18:01     ` Rob Clark
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2019-12-04 18:01 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Jordan Crouse,
	list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,,
	Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Linux Kernel Mailing List, Rob Herring,
	Will Deacon,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Wed, Dec 4, 2019 at 7:56 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 22/11/2019 11:31 pm, Jordan Crouse wrote:
> > Add a compatible string to identify SMMUs that are attached
> > to Adreno GPU devices that wish to support split pagetables.
>
> A software policy decision is not, in itself, a good justification for a
> DT property. Is the GPU SMMU fundamentally different in hardware* from
> the other SMMU(s) in any given SoC?

The GPU CP has some sort of mechanism to switch pagetables.. although
I guess under the firmware it is all the same.  Jordan should know
better..

BR,
-R

> (* where "hardware" may encompass hypervisor shenanigans)
>
> > Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> > ---
> >
> >   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > index 6515dbe..db9f826 100644
> > --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
> > @@ -31,6 +31,12 @@ properties:
> >                 - qcom,sdm845-smmu-v2
> >             - const: qcom,smmu-v2
> >
> > +      - description: Qcom Adreno GPU SMMU iplementing split pagetables
> > +        items:
> > +          - enum:
> > +              - qcom,adreno-smmu-v2
> > +          - const: qcom,smmu-v2
>
> Given that we already have per-SoC compatibles for Qcom SMMUs in
> general, this seems suspiciously vague.
>
> Robin.
>
> > +
> >         - description: Qcom SoCs implementing "arm,mmu-500"
> >           items:
> >             - enum:
> >
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations
  2019-12-04 16:44   ` [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations Robin Murphy
@ 2019-12-05 15:51     ` Jordan Crouse
  0 siblings, 0 replies; 16+ messages in thread
From: Jordan Crouse @ 2019-12-05 15:51 UTC (permalink / raw)
  To: Robin Murphy
  Cc: iommu, will, linux-arm-kernel, linux-arm-msm, linux-kernel, Joerg Roedel

On Wed, Dec 04, 2019 at 04:44:59PM +0000, Robin Murphy wrote:
> On 22/11/2019 11:31 pm, Jordan Crouse wrote:
> >Add implementation specific support to enable split pagetables for
> >SMMU implementations attached to Adreno GPUs on Qualcomm targets.
> >
> >To enable split pagetables the driver will set an attribute on the domain.
> >if conditions are correct, set up the hardware to support equally sized
> >TTBR0 and TTBR1 regions and programs the domain pagetable to TTBR1 to make
> >it available for global buffers while allowing the GPU the chance to
> >switch the TTBR0 at runtime for per-context pagetables.
> >
> >After programming the context, the value of the domain attribute can be
> >queried to see if split pagetables were successfully programmed. The
> >domain geometry will be updated so that the caller can determine the
> >start of the region to generate correct virtual addresses.
> 
> Why is any of this in impl? It all looks like perfectly generic
> architectural TTBR1 setup to me. As long as DOMAIN_ATTR_SPLIT_TABLES is
> explicitly an opt-in for callers, I'm OK with them having to trust that
> SEP_UPSTREAM is good enough. Or, even better, make the value of
> DOMAIN_ATTR_SPLIT_TABLES not a boolean but the actual split point, where the
> default of 0 would logically mean "no split".

(apologies if you get multiple copies of this email, I have tickets in with the
CAF IT folks).

I made it impl specific because my impression from the previous conversations
was that setting up the T0 space but leaving TTBR0 un-programmed was a silly
thing that was unique to the Adreno GPU. I don't mind moving it to the generic
code since that saves us from some silly compatible string games.

I like the idea of DOMAIN_ATTR_SPLIT_TABLES returning the split point but would
we want to allow the user to try to specific a desired split point ahead of
time? It is my impression that we only have a handful of valid SEP values and
I'm not sure what the right response would be if the user specified an incorrect
one.

So far I've not found a use for anything except SEP_UPSTREAM but I have the
extreme luxury of a SMMU with an actual 49 bit IAS.

New patchset coming soon.

Thanks,
Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization
       [not found] ` <0101016e95754002-c2fa43aa-b14b-4cff-b6f9-a67c96661e26-000000@us-west-2.amazonses.com>
@ 2019-12-09 20:11   ` Rob Clark
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2019-12-09 20:11 UTC (permalink / raw)
  To: Jordan Crouse
  Cc: list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,,
	Robin Murphy, Will Deacon,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-arm-msm, David Airlie, Sean Paul, Bruce Wang,
	AngeloGioacchino Del Regno, Sam Ravnborg, dri-devel,
	Jeykumar Sankaran, Thomas Gleixner, Jonathan Marek,
	Drew Davenport, Georgi Djakov, freedreno, Jeffrey Hugo,
	Mamta Shukla, Linux Kernel Mailing List, Daniel Vetter

On Fri, Nov 22, 2019 at 3:32 PM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> Everywhere an IOMMU object is created by msm_gpu_create_address_space
> the IOMMU device is attached immediately after. Instead of carrying around
> the infrastructure to do the attach from the device specific code do it
> directly in the msm_iommu_init() function. This gets it out of the way for
> more aggressive cleanups that follow.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Hmm, yeah, now that we dropped the extra mmu->attach() args (which was
some ancient downstream compat thing), we don't need the separate
attach step.  I suppose we probably should do a similar cleanup for
mmu->detach(), but I guess that can be it's own patch

Reviewed-by: Rob Clark <robdclark@gmail.com>


> ---
>
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  |  8 --------
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c |  4 ----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c |  7 -------
>  drivers/gpu/drm/msm/msm_gem_vma.c        | 23 +++++++++++++++++++----
>  drivers/gpu/drm/msm/msm_gpu.c            | 11 +----------
>  drivers/gpu/drm/msm/msm_gpummu.c         |  6 ------
>  drivers/gpu/drm/msm/msm_iommu.c          | 15 +++++++--------
>  drivers/gpu/drm/msm/msm_mmu.h            |  1 -
>  8 files changed, 27 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 6c92f0f..b082b23 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -704,7 +704,6 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
>  {
>         struct iommu_domain *domain;
>         struct msm_gem_address_space *aspace;
> -       int ret;
>
>         domain = iommu_domain_alloc(&platform_bus_type);
>         if (!domain)
> @@ -720,13 +719,6 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
>                 return PTR_ERR(aspace);
>         }
>
> -       ret = aspace->mmu->funcs->attach(aspace->mmu);
> -       if (ret) {
> -               DPU_ERROR("failed to attach iommu %d\n", ret);
> -               msm_gem_address_space_put(aspace);
> -               return ret;
> -       }
> -
>         dpu_kms->base.aspace = aspace;
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index dda0543..9dba37c 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -518,10 +518,6 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>                 }
>
>                 kms->aspace = aspace;
> -
> -               ret = aspace->mmu->funcs->attach(aspace->mmu);
> -               if (ret)
> -                       goto fail;
>         } else {
>                 DRM_DEV_INFO(dev->dev, "no iommu, fallback to phys "
>                                 "contig buffers for scanout\n");
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index e43ecd4..653dab2 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -736,13 +736,6 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
>                 }
>
>                 kms->aspace = aspace;
> -
> -               ret = aspace->mmu->funcs->attach(aspace->mmu);
> -               if (ret) {
> -                       DRM_DEV_ERROR(&pdev->dev, "failed to attach iommu: %d\n",
> -                               ret);
> -                       goto fail;
> -               }
>         } else {
>                 DRM_DEV_INFO(&pdev->dev,
>                          "no iommu, fallback to phys contig buffers for scanout\n");
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 1af5354..91d993a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -131,8 +131,8 @@ msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
>                 const char *name)
>  {
>         struct msm_gem_address_space *aspace;
> -       u64 size = domain->geometry.aperture_end -
> -               domain->geometry.aperture_start;
> +       u64 start = domain->geometry.aperture_start;
> +       u64 size = domain->geometry.aperture_end - start;
>
>         aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
>         if (!aspace)
> @@ -141,9 +141,18 @@ msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
>         spin_lock_init(&aspace->lock);
>         aspace->name = name;
>         aspace->mmu = msm_iommu_new(dev, domain);
> +       if (IS_ERR(aspace->mmu)) {
> +               int ret = PTR_ERR(aspace->mmu);
>
> -       drm_mm_init(&aspace->mm, (domain->geometry.aperture_start >> PAGE_SHIFT),
> -               size >> PAGE_SHIFT);
> +               kfree(aspace);
> +               return ERR_PTR(ret);
> +       }
> +
> +       /*
> +        * Attaching the IOMMU device changes the aperture values so use the
> +        * cached values instead
> +        */
> +       drm_mm_init(&aspace->mm, start >> PAGE_SHIFT, size >> PAGE_SHIFT);
>
>         kref_init(&aspace->kref);
>
> @@ -164,6 +173,12 @@ msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
>         spin_lock_init(&aspace->lock);
>         aspace->name = name;
>         aspace->mmu = msm_gpummu_new(dev, gpu);
> +       if (IS_ERR(aspace->mmu)) {
> +               int ret = PTR_ERR(aspace->mmu);
> +
> +               kfree(aspace);
> +               return ERR_PTR(ret);
> +       }
>
>         drm_mm_init(&aspace->mm, (va_start >> PAGE_SHIFT),
>                 size >> PAGE_SHIFT);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index 18f3a5c..f7bf80e 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -808,7 +808,6 @@ msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
>                 uint64_t va_start, uint64_t va_end)
>  {
>         struct msm_gem_address_space *aspace;
> -       int ret;
>
>         /*
>          * Setup IOMMU.. eventually we will (I think) do this once per context
> @@ -833,17 +832,9 @@ msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
>                         va_start, va_end);
>         }
>
> -       if (IS_ERR(aspace)) {
> +       if (IS_ERR(aspace))
>                 DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
>                         PTR_ERR(aspace));
> -               return ERR_CAST(aspace);
> -       }
> -
> -       ret = aspace->mmu->funcs->attach(aspace->mmu);
> -       if (ret) {
> -               msm_gem_address_space_put(aspace);
> -               return ERR_PTR(ret);
> -       }
>
>         return aspace;
>  }
> diff --git a/drivers/gpu/drm/msm/msm_gpummu.c b/drivers/gpu/drm/msm/msm_gpummu.c
> index 34980d8..0ad0f84 100644
> --- a/drivers/gpu/drm/msm/msm_gpummu.c
> +++ b/drivers/gpu/drm/msm/msm_gpummu.c
> @@ -21,11 +21,6 @@ struct msm_gpummu {
>  #define GPUMMU_PAGE_SIZE SZ_4K
>  #define TABLE_SIZE (sizeof(uint32_t) * GPUMMU_VA_RANGE / GPUMMU_PAGE_SIZE)
>
> -static int msm_gpummu_attach(struct msm_mmu *mmu)
> -{
> -       return 0;
> -}
> -
>  static void msm_gpummu_detach(struct msm_mmu *mmu)
>  {
>  }
> @@ -85,7 +80,6 @@ static void msm_gpummu_destroy(struct msm_mmu *mmu)
>  }
>
>  static const struct msm_mmu_funcs funcs = {
> -               .attach = msm_gpummu_attach,
>                 .detach = msm_gpummu_detach,
>                 .map = msm_gpummu_map,
>                 .unmap = msm_gpummu_unmap,
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index ad58cfe..544c519 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -23,13 +23,6 @@ static int msm_fault_handler(struct iommu_domain *domain, struct device *dev,
>         return 0;
>  }
>
> -static int msm_iommu_attach(struct msm_mmu *mmu)
> -{
> -       struct msm_iommu *iommu = to_msm_iommu(mmu);
> -
> -       return iommu_attach_device(iommu->domain, mmu->dev);
> -}
> -
>  static void msm_iommu_detach(struct msm_mmu *mmu)
>  {
>         struct msm_iommu *iommu = to_msm_iommu(mmu);
> @@ -66,7 +59,6 @@ static void msm_iommu_destroy(struct msm_mmu *mmu)
>  }
>
>  static const struct msm_mmu_funcs funcs = {
> -               .attach = msm_iommu_attach,
>                 .detach = msm_iommu_detach,
>                 .map = msm_iommu_map,
>                 .unmap = msm_iommu_unmap,
> @@ -76,6 +68,7 @@ static const struct msm_mmu_funcs funcs = {
>  struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
>  {
>         struct msm_iommu *iommu;
> +       int ret;
>
>         iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
>         if (!iommu)
> @@ -85,5 +78,11 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
>         msm_mmu_init(&iommu->base, dev, &funcs);
>         iommu_set_fault_handler(domain, msm_fault_handler, iommu);
>
> +       ret = iommu_attach_device(iommu->domain, dev);
> +       if (ret) {
> +               kfree(iommu);
> +               return ERR_PTR(ret);
> +       }
> +
>         return &iommu->base;
>  }
> diff --git a/drivers/gpu/drm/msm/msm_mmu.h b/drivers/gpu/drm/msm/msm_mmu.h
> index 67a623f..bae9e8e 100644
> --- a/drivers/gpu/drm/msm/msm_mmu.h
> +++ b/drivers/gpu/drm/msm/msm_mmu.h
> @@ -10,7 +10,6 @@
>  #include <linux/iommu.h>
>
>  struct msm_mmu_funcs {
> -       int (*attach)(struct msm_mmu *mmu);
>         void (*detach)(struct msm_mmu *mmu);
>         int (*map)(struct msm_mmu *mmu, uint64_t iova, struct sg_table *sgt,
>                         unsigned len, int prot);
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/8] drm/msm: Refactor address space initialization
       [not found] ` <0101016e95755c16-5ab6f6e7-83bb-4d01-b746-7cc6ea265ad7-000000@us-west-2.amazonses.com>
@ 2019-12-09 20:14   ` Rob Clark
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2019-12-09 20:14 UTC (permalink / raw)
  To: Jordan Crouse
  Cc: list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,,
	Robin Murphy, Will Deacon,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-arm-msm, Sean Paul, Sharat Masetty, dri-devel,
	Linux Kernel Mailing List, David Airlie, Allison Randal,
	Ben Dooks, Thomas Gleixner, Douglas Anderson, Greg Kroah-Hartman,
	Jeffrey Hugo, Wen Yang, Alexios Zavras, Jeykumar Sankaran,
	AngeloGioacchino Del Regno, Bruce Wang, Sam Ravnborg,
	Brian Masney, Jonathan Marek, Drew Davenport, Georgi Djakov,
	freedreno, Mamta Shukla, Daniel Vetter

On Fri, Nov 22, 2019 at 3:32 PM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> Refactor how address space initialization works. Instead of having the
> address space function create the MMU object (and thus require separate but
> equal functions for gpummu and iommu) use a single function and pass the
> MMU struct in. Make the generic code cleaner by using target specific
> functions to create the address space so a2xx can do its own thing in its
> own space.  For all the other targets use a generic helper to initialize
> IOMMU but leave the door open for newer targets to use customization
> if they need it.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
>
>  drivers/gpu/drm/msm/adreno/a2xx_gpu.c    | 16 ++++++++++
>  drivers/gpu/drm/msm/adreno/a3xx_gpu.c    |  1 +
>  drivers/gpu/drm/msm/adreno/a4xx_gpu.c    |  1 +
>  drivers/gpu/drm/msm/adreno/a5xx_gpu.c    |  1 +
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c    |  1 +
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c  | 23 ++++++++++----
>  drivers/gpu/drm/msm/adreno/adreno_gpu.h  |  8 +++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c  | 10 +++---
>  drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 14 +++++----
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c |  4 ---
>  drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 11 +++++--
>  drivers/gpu/drm/msm/msm_drv.h            |  8 ++---
>  drivers/gpu/drm/msm/msm_gem_vma.c        | 52 +++++---------------------------
>  drivers/gpu/drm/msm/msm_gpu.c            | 40 ++----------------------
>  drivers/gpu/drm/msm/msm_gpu.h            |  4 +--
>  drivers/gpu/drm/msm/msm_iommu.c          |  3 ++
>  16 files changed, 83 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
> index 1f83bc1..60f6472 100644
> --- a/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a2xx_gpu.c
> @@ -401,6 +401,21 @@ static struct msm_gpu_state *a2xx_gpu_state_get(struct msm_gpu *gpu)
>         return state;
>  }
>
> +static struct msm_gem_address_space *
> +a2xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev)
> +{
> +       struct msm_mmu *mmu = msm_gpummu_new(&pdev->dev, gpu);
> +       struct msm_gem_address_space *aspace;
> +
> +       aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
> +               SZ_16M + 0xfff * SZ_64K);
> +
> +       if (IS_ERR(aspace) && !IS_ERR(mmu))
> +               mmu->funcs->destroy(mmu);
> +
> +       return aspace;
> +}
> +
>  /* Register offset defines for A2XX - copy of A3XX */
>  static const unsigned int a2xx_register_offsets[REG_ADRENO_REGISTER_MAX] = {
>         REG_ADRENO_DEFINE(REG_ADRENO_CP_RB_BASE, REG_AXXX_CP_RB_BASE),
> @@ -429,6 +444,7 @@ static const struct adreno_gpu_funcs funcs = {
>  #endif
>                 .gpu_state_get = a2xx_gpu_state_get,
>                 .gpu_state_put = adreno_gpu_state_put,
> +               .create_address_space = a2xx_create_address_space,
>         },
>  };
>
> diff --git a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> index 7ad1493..41e51e0 100644
> --- a/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a3xx_gpu.c
> @@ -441,6 +441,7 @@ static const struct adreno_gpu_funcs funcs = {
>  #endif
>                 .gpu_state_get = a3xx_gpu_state_get,
>                 .gpu_state_put = adreno_gpu_state_put,
> +               .create_address_space = adreno_iommu_create_address_space,
>         },
>  };
>
> diff --git a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> index b01388a..3655440 100644
> --- a/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a4xx_gpu.c
> @@ -532,6 +532,7 @@ static const struct adreno_gpu_funcs funcs = {
>  #endif
>                 .gpu_state_get = a4xx_gpu_state_get,
>                 .gpu_state_put = adreno_gpu_state_put,
> +               .create_address_space = adreno_iommu_create_address_space,
>         },
>         .get_timestamp = a4xx_get_timestamp,
>  };
> diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> index b02e204..0f5db72 100644
> --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
> @@ -1432,6 +1432,7 @@ static const struct adreno_gpu_funcs funcs = {
>                 .gpu_busy = a5xx_gpu_busy,
>                 .gpu_state_get = a5xx_gpu_state_get,
>                 .gpu_state_put = a5xx_gpu_state_put,
> +               .create_address_space = adreno_iommu_create_address_space,
>         },
>         .get_timestamp = a5xx_get_timestamp,
>  };
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index dc8ec2c..5dc0b2c 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -832,6 +832,7 @@ static const struct adreno_gpu_funcs funcs = {
>  #if defined(CONFIG_DRM_MSM_GPU_STATE)
>                 .gpu_state_get = a6xx_gpu_state_get,
>                 .gpu_state_put = a6xx_gpu_state_put,
> +               .create_address_space = adreno_iommu_create_address_space,
>  #endif
>         },
>         .get_timestamp = a6xx_get_timestamp,
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 0783e4b..09c57891 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -157,6 +157,23 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)
>         return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, pasid);
>  }
>
> +struct msm_gem_address_space *
> +adreno_iommu_create_address_space(struct msm_gpu *gpu,
> +               struct platform_device *pdev)
> +{
> +       struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
> +       struct msm_mmu *mmu = msm_iommu_new(&pdev->dev, iommu);
> +       struct msm_gem_address_space *aspace;
> +
> +       aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
> +               0xfffffff);
> +
> +       if (IS_ERR(aspace) && !IS_ERR(mmu))
> +               mmu->funcs->destroy(mmu);
> +
> +       return aspace;
> +}
> +
>  int adreno_get_param(struct msm_gpu *gpu, uint32_t param, uint64_t *value)
>  {
>         struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> @@ -949,12 +966,6 @@ int adreno_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
>         adreno_gpu_config.ioname = "kgsl_3d0_reg_memory";
>
> -       adreno_gpu_config.va_start = SZ_16M;
> -       adreno_gpu_config.va_end = 0xffffffff;
> -       /* maximum range of a2xx mmu */
> -       if (adreno_is_a2xx(adreno_gpu))
> -               adreno_gpu_config.va_end = SZ_16M + 0xfff * SZ_64K;
> -
>         adreno_gpu_config.nr_rings = nr_rings;
>
>         adreno_get_pwrlevels(&pdev->dev, gpu);
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> index e71a757..5c1aa12 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> @@ -263,6 +263,14 @@ int adreno_gpu_state_get(struct msm_gpu *gpu, struct msm_gpu_state *state);
>  int adreno_gpu_state_put(struct msm_gpu_state *state);
>
>  /*
> + * Common helper function to initialize the default address space for arm-smmu
> + * attached targets
> + */
> +struct msm_gem_address_space *
> +adreno_iommu_create_address_space(struct msm_gpu *gpu,
> +               struct platform_device *pdev);
> +
> +/*
>   * For a5xx and a6xx targets load the zap shader that is used to pull the GPU
>   * out of secure mode
>   */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index b082b23..4e6ebbd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -704,18 +704,18 @@ static int _dpu_kms_mmu_init(struct dpu_kms *dpu_kms)
>  {
>         struct iommu_domain *domain;
>         struct msm_gem_address_space *aspace;
> +       struct msm_mmu *mmu;
>
>         domain = iommu_domain_alloc(&platform_bus_type);
>         if (!domain)
>                 return 0;
>
> -       domain->geometry.aperture_start = 0x1000;
> -       domain->geometry.aperture_end = 0xffffffff;
> +       mmu = msm_iommu_new(dpu_kms->dev->dev, domain);
> +       aspace = msm_gem_address_space_create(mmu, "dpu1",
> +               0x1000, 0xfffffff);
>
> -       aspace = msm_gem_address_space_create(dpu_kms->dev->dev,
> -                       domain, "dpu1");
>         if (IS_ERR(aspace)) {
> -               iommu_domain_free(domain);
> +               mmu->funcs->destroy(mmu);
>                 return PTR_ERR(aspace);
>         }
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> index 9dba37c..0889718 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c
> @@ -510,9 +510,15 @@ struct msm_kms *mdp4_kms_init(struct drm_device *dev)
>         mdelay(16);
>
>         if (config->iommu) {
> -               aspace = msm_gem_address_space_create(&pdev->dev,
> -                               config->iommu, "mdp4");
> +               struct msm_mmu *mmu = msm_iommu_new(&pdev->dev,
> +                       config->iommu);
> +
> +               aspace  = msm_gem_address_space_create(mmu,
> +                       "mdp4", 0x1000, 0xffffffff);
> +
>                 if (IS_ERR(aspace)) {
> +                       if (!IS_ERR(mmu))
> +                               mmu->funcs->destroy(mmu);
>                         ret = PTR_ERR(aspace);
>                         goto fail;
>                 }
> @@ -565,10 +571,6 @@ static struct mdp4_platform_config *mdp4_get_config(struct platform_device *dev)
>         /* TODO: Chips that aren't apq8064 have a 200 Mhz max_clk */
>         config.max_clk = 266667000;
>         config.iommu = iommu_domain_alloc(&platform_bus_type);
> -       if (config.iommu) {
> -               config.iommu->geometry.aperture_start = 0x1000;
> -               config.iommu->geometry.aperture_end = 0xffffffff;
> -       }
>
>         return &config;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> index 1f48f64..ebd651a 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_cfg.c
> @@ -941,10 +941,6 @@ static struct mdp5_cfg_platform *mdp5_get_config(struct platform_device *dev)
>         static struct mdp5_cfg_platform config = {};
>
>         config.iommu = iommu_domain_alloc(&platform_bus_type);
> -       if (config.iommu) {
> -               config.iommu->geometry.aperture_start = 0x1000;
> -               config.iommu->geometry.aperture_end = 0xffffffff;
> -       }
>
>         return &config;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 653dab2..20bdff9 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -724,13 +724,20 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
>         mdelay(16);
>
>         if (config->platform.iommu) {
> +               struct msm_mmu *mmu;
> +
>                 iommu_dev = &pdev->dev;
>                 if (!iommu_dev->iommu_fwspec)
>                         iommu_dev = iommu_dev->parent;
>
> -               aspace = msm_gem_address_space_create(iommu_dev,
> -                               config->platform.iommu, "mdp5");
> +               mmu = msm_iommu_new(iommu_dev, config->platform.iommu);
> +
> +               aspace = msm_gem_address_space_create(mmu, "mdp5",
> +                       0x1000, 0xffffffff);
> +
>                 if (IS_ERR(aspace)) {
> +                       if (!IS_ERR(mmu))
> +                               mmu->funcs->destroy(mmu);
>                         ret = PTR_ERR(aspace);
>                         goto fail;
>                 }
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 71547e7..2203729 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -247,12 +247,8 @@ void msm_gem_close_vma(struct msm_gem_address_space *aspace,
>  void msm_gem_address_space_put(struct msm_gem_address_space *aspace);
>
>  struct msm_gem_address_space *
> -msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
> -               const char *name);
> -
> -struct msm_gem_address_space *
> -msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
> -               const char *name, uint64_t va_start, uint64_t va_end);
> +msm_gem_address_space_create(struct msm_mmu *mmu, const char *name,
> +               u64 va_start, u64 va_end);
>
>  int msm_register_mmu(struct drm_device *dev, struct msm_mmu *mmu);
>  void msm_unregister_mmu(struct drm_device *dev, struct msm_mmu *mmu);
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 91d993a..075ce52 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -125,63 +125,25 @@ int msm_gem_init_vma(struct msm_gem_address_space *aspace,
>         return 0;
>  }
>
> -
>  struct msm_gem_address_space *
> -msm_gem_address_space_create(struct device *dev, struct iommu_domain *domain,
> -               const char *name)
> -{
> -       struct msm_gem_address_space *aspace;
> -       u64 start = domain->geometry.aperture_start;
> -       u64 size = domain->geometry.aperture_end - start;
> -
> -       aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
> -       if (!aspace)
> -               return ERR_PTR(-ENOMEM);
> -
> -       spin_lock_init(&aspace->lock);
> -       aspace->name = name;
> -       aspace->mmu = msm_iommu_new(dev, domain);
> -       if (IS_ERR(aspace->mmu)) {
> -               int ret = PTR_ERR(aspace->mmu);
> -
> -               kfree(aspace);
> -               return ERR_PTR(ret);
> -       }
> -
> -       /*
> -        * Attaching the IOMMU device changes the aperture values so use the
> -        * cached values instead
> -        */
> -       drm_mm_init(&aspace->mm, start >> PAGE_SHIFT, size >> PAGE_SHIFT);
> -
> -       kref_init(&aspace->kref);
> -
> -       return aspace;
> -}
> -
> -struct msm_gem_address_space *
> -msm_gem_address_space_create_a2xx(struct device *dev, struct msm_gpu *gpu,
> -               const char *name, uint64_t va_start, uint64_t va_end)
> +msm_gem_address_space_create(struct msm_mmu *mmu, const char *name,
> +               u64 va_start, u64 va_end)
>  {
>         struct msm_gem_address_space *aspace;
>         u64 size = va_end - va_start;
>
> +       if (IS_ERR(mmu))
> +               return ERR_CAST(mmu);
> +
>         aspace = kzalloc(sizeof(*aspace), GFP_KERNEL);
>         if (!aspace)
>                 return ERR_PTR(-ENOMEM);
>
>         spin_lock_init(&aspace->lock);
>         aspace->name = name;
> -       aspace->mmu = msm_gpummu_new(dev, gpu);
> -       if (IS_ERR(aspace->mmu)) {
> -               int ret = PTR_ERR(aspace->mmu);
> -
> -               kfree(aspace);
> -               return ERR_PTR(ret);
> -       }
> +       aspace->mmu = mmu;
>
> -       drm_mm_init(&aspace->mm, (va_start >> PAGE_SHIFT),
> -               size >> PAGE_SHIFT);
> +       drm_mm_init(&aspace->mm, va_start >> PAGE_SHIFT, size >> PAGE_SHIFT);
>
>         kref_init(&aspace->kref);
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index f7bf80e..f11df53 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -803,42 +803,6 @@ static int get_clocks(struct platform_device *pdev, struct msm_gpu *gpu)
>         return 0;
>  }
>
> -static struct msm_gem_address_space *
> -msm_gpu_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev,
> -               uint64_t va_start, uint64_t va_end)
> -{
> -       struct msm_gem_address_space *aspace;
> -
> -       /*
> -        * Setup IOMMU.. eventually we will (I think) do this once per context
> -        * and have separate page tables per context.  For now, to keep things
> -        * simple and to get something working, just use a single address space:
> -        */
> -       if (!adreno_is_a2xx(to_adreno_gpu(gpu))) {
> -               struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
> -               if (!iommu)
> -                       return NULL;
> -
> -               iommu->geometry.aperture_start = va_start;
> -               iommu->geometry.aperture_end = va_end;
> -
> -               DRM_DEV_INFO(gpu->dev->dev, "%s: using IOMMU\n", gpu->name);
> -
> -               aspace = msm_gem_address_space_create(&pdev->dev, iommu, "gpu");
> -               if (IS_ERR(aspace))
> -                       iommu_domain_free(iommu);
> -       } else {
> -               aspace = msm_gem_address_space_create_a2xx(&pdev->dev, gpu, "gpu",
> -                       va_start, va_end);
> -       }
> -
> -       if (IS_ERR(aspace))
> -               DRM_DEV_ERROR(gpu->dev->dev, "failed to init mmu: %ld\n",
> -                       PTR_ERR(aspace));
> -
> -       return aspace;
> -}
> -
>  int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>                 struct msm_gpu *gpu, const struct msm_gpu_funcs *funcs,
>                 const char *name, struct msm_gpu_config *config)
> @@ -911,8 +875,8 @@ int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
>
>         msm_devfreq_init(gpu);
>
> -       gpu->aspace = msm_gpu_create_address_space(gpu, pdev,
> -               config->va_start, config->va_end);
> +
> +       gpu->aspace = gpu->funcs->create_address_space(gpu, pdev);
>
>         if (gpu->aspace == NULL)
>                 DRM_DEV_INFO(drm->dev, "%s: no IOMMU, fallback to VRAM carveout!\n", name);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index ab8f0f9c..41d86c2 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -21,8 +21,6 @@ struct msm_gpu_state;
>
>  struct msm_gpu_config {
>         const char *ioname;
> -       uint64_t va_start;
> -       uint64_t va_end;
>         unsigned int nr_rings;
>  };
>
> @@ -64,6 +62,8 @@ struct msm_gpu_funcs {
>         int (*gpu_state_put)(struct msm_gpu_state *state);
>         unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
>         void (*gpu_set_freq)(struct msm_gpu *gpu, unsigned long freq);
> +       struct msm_gem_address_space *(*create_address_space)
> +               (struct msm_gpu *gpu, struct platform_device *pdev);
>  };
>
>  struct msm_gpu {
> diff --git a/drivers/gpu/drm/msm/msm_iommu.c b/drivers/gpu/drm/msm/msm_iommu.c
> index 544c519..e773ef8 100644
> --- a/drivers/gpu/drm/msm/msm_iommu.c
> +++ b/drivers/gpu/drm/msm/msm_iommu.c
> @@ -70,6 +70,9 @@ struct msm_mmu *msm_iommu_new(struct device *dev, struct iommu_domain *domain)
>         struct msm_iommu *iommu;
>         int ret;
>
> +       if (!domain)
> +               return ERR_PTR(-ENODEV);
> +
>         iommu = kzalloc(sizeof(*iommu), GFP_KERNEL);
>         if (!iommu)
>                 return ERR_PTR(-ENOMEM);
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables
       [not found] ` <0101016e95754ea7-d6414f4c-9e25-4bc4-a852-7116a783bf63-000000@us-west-2.amazonses.com>
@ 2019-12-09 20:17   ` Rob Clark
  0 siblings, 0 replies; 16+ messages in thread
From: Rob Clark @ 2019-12-09 20:17 UTC (permalink / raw)
  To: Jordan Crouse
  Cc: list@263.net:IOMMU DRIVERS
	<iommu@lists.linux-foundation.org>,
	Joerg Roedel <joro@8bytes.org>,,
	Robin Murphy, Will Deacon,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	linux-arm-msm, Sean Paul, Linux Kernel Mailing List, dri-devel,
	David Airlie, freedreno, Mamta Shukla, Daniel Vetter

On Fri, Nov 22, 2019 at 3:32 PM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> Attempt to enable split pagetables if the arm-smmu driver supports it.
> This will move the default address space from the default region to
> the address range assigned to TTBR1. The behavior should be transparent
> to the driver for now but it gets the default buffers out of the way
> when we want to start swapping TTBR0 for context-specific pagetables.
>
> Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
> ---
>
>  drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index 5dc0b2c..96b3b28 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -811,6 +811,50 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
>         return (unsigned long)busy_time;
>  }
>
> +static struct msm_gem_address_space *
> +a6xx_create_address_space(struct msm_gpu *gpu, struct platform_device *pdev)
> +{
> +       struct iommu_domain *iommu = iommu_domain_alloc(&platform_bus_type);
> +       struct msm_gem_address_space *aspace;
> +       struct msm_mmu *mmu;
> +       u64 start, size;
> +       u32 val = 1;
> +       int ret;
> +
> +       if (!iommu)
> +               return ERR_PTR(-ENOMEM);
> +
> +       /* Try to request split pagetables */
> +       iommu_domain_set_attr(iommu, DOMAIN_ATTR_SPLIT_TABLES, &val);
> +
> +       mmu = msm_iommu_new(&pdev->dev, iommu);
> +       if (IS_ERR(mmu)) {
> +               iommu_domain_free(iommu);
> +               return ERR_CAST(mmu);
> +       }
> +
> +       /* Check to see if split pagetables were successful */
> +       ret = iommu_domain_get_attr(iommu, DOMAIN_ATTR_SPLIT_TABLES, &val);

I assume the split between this and iommu_domain_set_attr() is because
attach needs to happen in between?  At any rate, if it needs to be
like this, maybe a comment is in order.  As it is it looks like
something future-self would "cleanup"..

BR,
-R

> +       if (!ret && val) {
> +               /*
> +                * The aperture start will be at the beginning of the TTBR1
> +                * space so use that as a base
> +                */
> +               start = iommu->geometry.aperture_start;
> +               size = 0xffffffff;
> +       } else {
> +               /* Otherwise use the legacy 32 bit region */
> +               start = SZ_16M;
> +               size = 0xffffffff - SZ_16M;
> +       }
> +
> +       aspace = msm_gem_address_space_create(mmu, "gpu", start, size);
> +       if (IS_ERR(aspace))
> +               iommu_domain_free(iommu);
> +
> +       return aspace;
> +}
> +
>  static const struct adreno_gpu_funcs funcs = {
>         .base = {
>                 .get_param = adreno_get_param,
> @@ -832,7 +876,7 @@ static const struct adreno_gpu_funcs funcs = {
>  #if defined(CONFIG_DRM_MSM_GPU_STATE)
>                 .gpu_state_get = a6xx_gpu_state_get,
>                 .gpu_state_put = a6xx_gpu_state_put,
> -               .create_address_space = adreno_iommu_create_address_space,
> +               .create_address_space = a6xx_create_address_space,
>  #endif
>         },
>         .get_timestamp = a6xx_get_timestamp,
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-12-09 20:18 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1574465484-7115-1-git-send-email-jcrouse@codeaurora.org>
2019-11-22 23:31 ` [PATCH v2 2/8] iommu: Add DOMAIN_ATTR_SPLIT_TABLES Jordan Crouse
2019-11-22 23:31 ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Jordan Crouse
2019-12-03 19:14   ` Rob Herring
2019-11-22 23:31 ` [PATCH v2 3/8] iommu/arm-smmu: Pass io_pgtable_cfg to impl specific init_context Jordan Crouse
2019-11-22 23:31 ` [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations Jordan Crouse
2019-11-22 23:31 ` [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization Jordan Crouse
2019-11-22 23:32 ` [PATCH v2 8/8] arm64: dts: qcom: sdm845: Update Adreno GPU SMMU compatible string Jordan Crouse
2019-11-22 23:32 ` [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables Jordan Crouse
2019-11-22 23:32 ` [PATCH v2 6/8] drm/msm: Refactor address space initialization Jordan Crouse
     [not found] ` <0101016e95751c0b-33c9379b-6b8c-43b1-8785-e5e1b6f084f1-000000@us-west-2.amazonses.com>
2019-12-04 15:55   ` [PATCH v2 1/8] dt-bindings: arm-smmu: Add Adreno GPU variant Robin Murphy
2019-12-04 18:01     ` Rob Clark
     [not found] ` <0101016e95752703-78491f46-41db-441c-b0fb-9a760e4d56cb-000000@us-west-2.amazonses.com>
2019-12-04 16:44   ` [PATCH v2 4/8] iommu/arm-smmu: Add split pagetables for Adreno IOMMU implementations Robin Murphy
2019-12-05 15:51     ` Jordan Crouse
     [not found] ` <0101016e95754002-c2fa43aa-b14b-4cff-b6f9-a67c96661e26-000000@us-west-2.amazonses.com>
2019-12-09 20:11   ` [PATCH v2 5/8] drm/msm: Attach the IOMMU device during initialization Rob Clark
     [not found] ` <0101016e95755c16-5ab6f6e7-83bb-4d01-b746-7cc6ea265ad7-000000@us-west-2.amazonses.com>
2019-12-09 20:14   ` [PATCH v2 6/8] drm/msm: Refactor address space initialization Rob Clark
     [not found] ` <0101016e95754ea7-d6414f4c-9e25-4bc4-a852-7116a783bf63-000000@us-west-2.amazonses.com>
2019-12-09 20:17   ` [PATCH v2 7/8] drm/msm/a6xx: Support split pagetables Rob Clark

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).