On Thu, Jan 2, 2020 at 3:02 AM Sharat Masetty <smasetty@xxxxxxxxxxxxxx> wrote: > > From: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> > > Add iommu domain attribute for using system cache aka last level > cache on QCOM SoCs by client drivers like GPU to set right > attributes for caching the hardware pagetables into the system cache. > > Signed-off-by: Vivek Gautam <vivek.gautam@xxxxxxxxxxxxxx> > Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx> > --- > drivers/iommu/arm-smmu-qcom.c | 10 ++++++++++ > drivers/iommu/arm-smmu.c | 14 ++++++++++++++ > drivers/iommu/arm-smmu.h | 1 + > include/linux/iommu.h | 1 + > 4 files changed, 26 insertions(+) > > diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c > index 24c071c..d1d22df 100644 > --- a/drivers/iommu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm-smmu-qcom.c > @@ -30,7 +30,17 @@ static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > return ret; > } > > +static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain, > + struct io_pgtable_cfg *pgtbl_cfg) > +{ > + if (smmu_domain->sys_cache) > + pgtbl_cfg->coherent_walk = false; just curious, does coherent walk not work with sys-cache, or is it just that it is kind of pointless (given that, afaiu, the pagetables can be cached by the system cache)? > + > + return 0; > +} > + > static const struct arm_smmu_impl qcom_smmu_impl = { > + .init_context = qcom_smmu_init_context, > .reset = qcom_sdm845_smmu500_reset, > }; > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 4f7e0c0..055b548 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -1466,6 +1466,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_QCOM_SYS_CACHE: > + *((int *)data) = smmu_domain->sys_cache; > + return 0; > default: > return -ENODEV; > } > @@ -1506,6 +1509,17 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain, > else > smmu_domain->stage = ARM_SMMU_DOMAIN_S1; > break; > + case DOMAIN_ATTR_QCOM_SYS_CACHE: > + if (smmu_domain->smmu) { > + ret = -EPERM; > + goto out_unlock; > + } > + > + if (*((int *)data)) > + smmu_domain->sys_cache = true; > + else > + smmu_domain->sys_cache = false; > + break; > default: > ret = -ENODEV; > } > diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h > index f57cdbe..8aeaaf0 100644 > --- a/drivers/iommu/arm-smmu.h > +++ b/drivers/iommu/arm-smmu.h > @@ -322,6 +322,7 @@ 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 sys_cache; > }; > > > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index 0c60e75..bd61c60 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -127,6 +127,7 @@ enum iommu_attr { > DOMAIN_ATTR_FSL_PAMUV1, > DOMAIN_ATTR_NESTING, /* two stages of translation */ > DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE, > + DOMAIN_ATTR_QCOM_SYS_CACHE, Given that IOMMU_QCOM_SYS_CACHE was renamed to IOMMU_SYS_CACHE_ONLY, I wonder if this domain attr should simply be DOMAIN_ATTR_SYS_CACHE? But that said, the function of this domain attr seems to simply be to disable coherent walk.. I wonder if naming the domain attr after what it does would make more sense? BR, -R > DOMAIN_ATTR_MAX, > }; > > -- > 1.9.1 > _______________________________________________ > Freedreno mailing list > Freedreno@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/freedreno