This patch series consists of three parts and covers the following:
1. Introducing intital set of driver changes to implement ACTLR register
for custom prefetcher settings in Qualcomm SoCs.
2. Adding ACTLR data and implementation operations for SM8550.
3. Re-enabling context caching for Qualcomm SoCs to retain prefetcher
settings during reset and runtime suspend.
Link to discussion on RFC posted on ACTLR implementation:
https://lore.kernel.org/all/[email protected]/
Bibek Kumar Patro (3):
iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings
iommu/arm-smmu: add ACTLR data and support for sm8550
iommu/arm-smmu: re-enable context caching in smmu reset operation
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 143 ++++++++++++++++++++-
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 +
drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 +
4 files changed, 146 insertions(+), 9 deletions(-)
--
2.17.1
Currently in Qualcomm SoCs the default prefetch is set to 1 which allows
the TLB to fetch just the next page table. MMU-500 features ACTLR
register which is implementation defined and is used for Qualcomm SoCs
to have a prefetch setting of 1/3/7/15 enabling TLB to prefetch
the next set of page tables accordingly allowing for faster translations.
ACTLR value is unique for each SMR (Stream matching register) and stored
in a pre-populated table. This value is set to the register during
context bank initialisation.
Signed-off-by: Bibek Kumar Patro <[email protected]>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 34 ++++++++++++++++++++++
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 ++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 ++--
drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++++
4 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index ae7cae015193..68c1f4908473 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -14,6 +14,17 @@
#define QCOM_DUMMY_VAL -1
+struct actlr_config {
+ const struct actlr_data *adata;
+ u32 size;
+};
+
+struct actlr_data {
+ u16 sid;
+ u16 mask;
+ u32 actlr;
+};
+
static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
{
return container_of(smmu, struct qcom_smmu, smmu);
@@ -270,6 +281,26 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+ const struct actlr_config *actlrcfg;
+ struct arm_smmu_smr *smr = smmu->smrs;
+ int idx = smmu_domain->cfg.cbndx;
+ int i;
+ u16 id;
+ u16 mask;
+
+ if (qsmmu->actlrcfg) {
+ actlrcfg = qsmmu->actlrcfg;
+ for (i = 0; i < actlrcfg->size; ++i) {
+ id = actlrcfg->adata[i].sid;
+ mask = actlrcfg->adata[i].mask;
+ if (!smr_is_subset(*smr, id, mask))
+ arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR,
+ actlrcfg->adata[i].actlr);
+ }
+ }
+
smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
return 0;
@@ -459,6 +490,9 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
qsmmu->smmu.impl = impl;
qsmmu->cfg = data->cfg;
+ if (data->actlrcfg && (data->actlrcfg->size))
+ qsmmu->actlrcfg = data->actlrcfg;
+
return &qsmmu->smmu;
}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
index 593910567b88..4b6862715070 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
@@ -9,6 +9,7 @@
struct qcom_smmu {
struct arm_smmu_device smmu;
const struct qcom_smmu_config *cfg;
+ const struct actlr_config *actlrcfg;
bool bypass_quirk;
u8 bypass_cbndx;
u32 stall_enabled;
@@ -25,6 +26,7 @@ struct qcom_smmu_config {
};
struct qcom_smmu_match_data {
+ const struct actlr_config *actlrcfg;
const struct qcom_smmu_config *cfg;
const struct arm_smmu_impl *impl;
const struct arm_smmu_impl *adreno_impl;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4c79ef6f4c75..38ac1cbc799b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -992,9 +992,10 @@ static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask)
* expect simply identical entries for this case, but there's
* no harm in accommodating the generalisation.
*/
- if ((mask & smrs[i].mask) == mask &&
- !((id ^ smrs[i].id) & ~smrs[i].mask))
+
+ if (smr_is_subset(smrs[i], id, mask))
return i;
+
/*
* If the new entry has any other overlap with an existing one,
* though, then there always exists at least one stream ID
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 703fd5817ec1..b1638bbc41d4 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -501,6 +501,11 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
}
+static inline bool smr_is_subset(struct arm_smmu_smr smrs, u16 id, u16 mask)
+{
+ return (mask & smrs.mask) == mask && !((id ^ smrs.id) & ~smrs.mask);
+}
+
#define ARM_SMMU_GR0 0
#define ARM_SMMU_GR1 1
#define ARM_SMMU_CB(s, n) ((s)->numpage + (n))
--
2.17.1
Add ACTLR data table for SM8550 along with support for
same including SM8550 specific implementation operations.
Signed-off-by: Bibek Kumar Patro <[email protected]>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
1 file changed, 81 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 68c1f4908473..590b7c285299 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -25,6 +25,64 @@ struct actlr_data {
u32 actlr;
};
+static const struct actlr_data sm8550_apps_actlr_data[] = {
+ { 0x18a0, 0x0000, 0x00000103 },
+ { 0x18e0, 0x0000, 0x00000103 },
+ { 0x0800, 0x0020, 0x00000001 },
+ { 0x1800, 0x00c0, 0x00000001 },
+ { 0x1820, 0x0000, 0x00000001 },
+ { 0x1860, 0x0000, 0x00000001 },
+ { 0x0c01, 0x0020, 0x00000303 },
+ { 0x0c02, 0x0020, 0x00000303 },
+ { 0x0c03, 0x0020, 0x00000303 },
+ { 0x0c04, 0x0020, 0x00000303 },
+ { 0x0c05, 0x0020, 0x00000303 },
+ { 0x0c06, 0x0020, 0x00000303 },
+ { 0x0c07, 0x0020, 0x00000303 },
+ { 0x0c08, 0x0020, 0x00000303 },
+ { 0x0c09, 0x0020, 0x00000303 },
+ { 0x0c0c, 0x0020, 0x00000303 },
+ { 0x0c0d, 0x0020, 0x00000303 },
+ { 0x0c0e, 0x0020, 0x00000303 },
+ { 0x0c0f, 0x0020, 0x00000303 },
+ { 0x1961, 0x0000, 0x00000303 },
+ { 0x1962, 0x0000, 0x00000303 },
+ { 0x1963, 0x0000, 0x00000303 },
+ { 0x1964, 0x0000, 0x00000303 },
+ { 0x1965, 0x0000, 0x00000303 },
+ { 0x1966, 0x0000, 0x00000303 },
+ { 0x1967, 0x0000, 0x00000303 },
+ { 0x1968, 0x0000, 0x00000303 },
+ { 0x1969, 0x0000, 0x00000303 },
+ { 0x196c, 0x0000, 0x00000303 },
+ { 0x196d, 0x0000, 0x00000303 },
+ { 0x196e, 0x0000, 0x00000303 },
+ { 0x196f, 0x0000, 0x00000303 },
+ { 0x19c1, 0x0010, 0x00000303 },
+ { 0x19c2, 0x0010, 0x00000303 },
+ { 0x19c3, 0x0010, 0x00000303 },
+ { 0x19c4, 0x0010, 0x00000303 },
+ { 0x19c5, 0x0010, 0x00000303 },
+ { 0x19c6, 0x0010, 0x00000303 },
+ { 0x19c7, 0x0010, 0x00000303 },
+ { 0x19c8, 0x0010, 0x00000303 },
+ { 0x19c9, 0x0010, 0x00000303 },
+ { 0x19cc, 0x0010, 0x00000303 },
+ { 0x19cd, 0x0010, 0x00000303 },
+ { 0x19ce, 0x0010, 0x00000303 },
+ { 0x19cf, 0x0010, 0x00000303 },
+ { 0x1c00, 0x0002, 0x00000103 },
+ { 0x1c01, 0x0000, 0x00000001 },
+ { 0x1920, 0x0000, 0x00000103 },
+ { 0x1923, 0x0000, 0x00000103 },
+ { 0x1924, 0x0000, 0x00000103 },
+ { 0x1940, 0x0000, 0x00000103 },
+ { 0x1941, 0x0004, 0x00000103 },
+ { 0x1943, 0x0000, 0x00000103 },
+ { 0x1944, 0x0000, 0x00000103 },
+ { 0x1947, 0x0000, 0x00000103 },
+};
+
static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
{
return container_of(smmu, struct qcom_smmu, smmu);
@@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
.tlb_sync = qcom_smmu_tlb_sync,
};
+
+static const struct arm_smmu_impl sm8550_smmu_500_impl = {
+ .init_context = qcom_smmu_init_context,
+ .cfg_probe = qcom_smmu_cfg_probe,
+ .def_domain_type = qcom_smmu_def_domain_type,
+ .reset = arm_mmu500_reset,
+ .write_s2cr = qcom_smmu_write_s2cr,
+ .tlb_sync = qcom_smmu_tlb_sync,
+};
+
static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
.init_context = qcom_adreno_smmu_init_context,
.def_domain_type = qcom_smmu_def_domain_type,
@@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
.reg_offset = qcom_smmu_impl0_reg_offset,
};
+static const struct actlr_config sm8550_actlrcfg = {
+ .adata = sm8550_apps_actlr_data,
+ .size = ARRAY_SIZE(sm8550_apps_actlr_data),
+};
+
/*
* It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
* there are not enough context banks.
@@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
/* Also no debug configuration. */
};
+
+static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
+ .impl = &sm8550_smmu_500_impl,
+ .adreno_impl = &qcom_adreno_smmu_500_impl,
+ .cfg = &qcom_smmu_impl0_cfg,
+ .actlrcfg = &sm8550_actlrcfg,
+};
+
static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
.impl = &qcom_smmu_500_impl,
.adreno_impl = &qcom_adreno_smmu_500_impl,
.cfg = &qcom_smmu_impl0_cfg,
};
-/*
- * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
- * special handling and can not be covered by the qcom,smmu-500 entry.
- */
static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
{ .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
{ .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
--
2.17.1
Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
through SoC specific reset ops, which is disabled in the default MMU-500
reset ops, but is expected for context banks using ACTLR register to
retain the prefetch value during reset and runtime suspend.
Signed-off-by: Bibek Kumar Patro <[email protected]>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 590b7c285299..f342b4778cf1 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
return match ? IOMMU_DOMAIN_IDENTITY : 0;
}
+#define ARM_MMU500_ACTLR_CPRE BIT(1)
+
+static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
+{
+ int i;
+ u32 reg;
+
+ arm_mmu500_reset(smmu);
+
+ for (i = 0; i < smmu->num_context_banks; ++i) {
+ reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
+ reg |= ARM_MMU500_ACTLR_CPRE;
+ arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg);
+ }
+
+ return 0;
+}
+
static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
{
int ret;
- arm_mmu500_reset(smmu);
+ qcom_smmu500_reset(smmu);
/*
* To address performance degradation in non-real time clients,
@@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = {
.init_context = qcom_smmu_init_context,
.cfg_probe = qcom_smmu_cfg_probe,
.def_domain_type = qcom_smmu_def_domain_type,
- .reset = arm_mmu500_reset,
+ .reset = qcom_smmu500_reset,
.write_s2cr = qcom_smmu_write_s2cr,
.tlb_sync = qcom_smmu_tlb_sync,
};
@@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = {
.init_context = qcom_smmu_init_context,
.cfg_probe = qcom_smmu_cfg_probe,
.def_domain_type = qcom_smmu_def_domain_type,
- .reset = arm_mmu500_reset,
+ .reset = qcom_smmu500_reset,
.write_s2cr = qcom_smmu_write_s2cr,
.tlb_sync = qcom_smmu_tlb_sync,
};
@@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = {
.init_context = qcom_adreno_smmu_init_context,
.def_domain_type = qcom_smmu_def_domain_type,
- .reset = arm_mmu500_reset,
+ .reset = qcom_smmu500_reset,
.alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
.write_sctlr = qcom_adreno_smmu_write_sctlr,
.tlb_sync = qcom_smmu_tlb_sync,
--
2.17.1
On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
<[email protected]> wrote:
>
> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
> through SoC specific reset ops, which is disabled in the default MMU-500
> reset ops, but is expected for context banks using ACTLR register to
> retain the prefetch value during reset and runtime suspend.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 590b7c285299..f342b4778cf1 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
> return match ? IOMMU_DOMAIN_IDENTITY : 0;
> }
>
> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
> +
> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
> +{
> + int i;
> + u32 reg;
> +
> + arm_mmu500_reset(smmu);
> +
> + for (i = 0; i < smmu->num_context_banks; ++i) {
> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
> + reg |= ARM_MMU500_ACTLR_CPRE;
> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg);
> + }
Wrong indentation. Did you run your patches through checkpatch.pl?
> +
> + return 0;
> +}
> +
> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
> {
> int ret;
>
> - arm_mmu500_reset(smmu);
> + qcom_smmu500_reset(smmu);
>
> /*
> * To address performance degradation in non-real time clients,
> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = {
> .init_context = qcom_smmu_init_context,
> .cfg_probe = qcom_smmu_cfg_probe,
> .def_domain_type = qcom_smmu_def_domain_type,
> - .reset = arm_mmu500_reset,
> + .reset = qcom_smmu500_reset,
> .write_s2cr = qcom_smmu_write_s2cr,
> .tlb_sync = qcom_smmu_tlb_sync,
> };
> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> .init_context = qcom_smmu_init_context,
> .cfg_probe = qcom_smmu_cfg_probe,
> .def_domain_type = qcom_smmu_def_domain_type,
> - .reset = arm_mmu500_reset,
> + .reset = qcom_smmu500_reset,
> .write_s2cr = qcom_smmu_write_s2cr,
> .tlb_sync = qcom_smmu_tlb_sync,
> };
> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = {
> .init_context = qcom_adreno_smmu_init_context,
> .def_domain_type = qcom_smmu_def_domain_type,
> - .reset = arm_mmu500_reset,
> + .reset = qcom_smmu500_reset,
> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
> .write_sctlr = qcom_adreno_smmu_write_sctlr,
> .tlb_sync = qcom_smmu_tlb_sync,
> --
> 2.17.1
>
--
With best wishes
Dmitry
On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
<[email protected]> wrote:
>
> Add ACTLR data table for SM8550 along with support for
> same including SM8550 specific implementation operations.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
> 1 file changed, 81 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 68c1f4908473..590b7c285299 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -25,6 +25,64 @@ struct actlr_data {
> u32 actlr;
> };
>
> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> + { 0x18a0, 0x0000, 0x00000103 },
> + { 0x18e0, 0x0000, 0x00000103 },
> + { 0x0800, 0x0020, 0x00000001 },
> + { 0x1800, 0x00c0, 0x00000001 },
> + { 0x1820, 0x0000, 0x00000001 },
> + { 0x1860, 0x0000, 0x00000001 },
> + { 0x0c01, 0x0020, 0x00000303 },
> + { 0x0c02, 0x0020, 0x00000303 },
> + { 0x0c03, 0x0020, 0x00000303 },
> + { 0x0c04, 0x0020, 0x00000303 },
> + { 0x0c05, 0x0020, 0x00000303 },
> + { 0x0c06, 0x0020, 0x00000303 },
> + { 0x0c07, 0x0020, 0x00000303 },
> + { 0x0c08, 0x0020, 0x00000303 },
> + { 0x0c09, 0x0020, 0x00000303 },
> + { 0x0c0c, 0x0020, 0x00000303 },
> + { 0x0c0d, 0x0020, 0x00000303 },
> + { 0x0c0e, 0x0020, 0x00000303 },
> + { 0x0c0f, 0x0020, 0x00000303 },
> + { 0x1961, 0x0000, 0x00000303 },
> + { 0x1962, 0x0000, 0x00000303 },
> + { 0x1963, 0x0000, 0x00000303 },
> + { 0x1964, 0x0000, 0x00000303 },
> + { 0x1965, 0x0000, 0x00000303 },
> + { 0x1966, 0x0000, 0x00000303 },
> + { 0x1967, 0x0000, 0x00000303 },
> + { 0x1968, 0x0000, 0x00000303 },
> + { 0x1969, 0x0000, 0x00000303 },
> + { 0x196c, 0x0000, 0x00000303 },
> + { 0x196d, 0x0000, 0x00000303 },
> + { 0x196e, 0x0000, 0x00000303 },
> + { 0x196f, 0x0000, 0x00000303 },
> + { 0x19c1, 0x0010, 0x00000303 },
> + { 0x19c2, 0x0010, 0x00000303 },
> + { 0x19c3, 0x0010, 0x00000303 },
> + { 0x19c4, 0x0010, 0x00000303 },
> + { 0x19c5, 0x0010, 0x00000303 },
> + { 0x19c6, 0x0010, 0x00000303 },
> + { 0x19c7, 0x0010, 0x00000303 },
> + { 0x19c8, 0x0010, 0x00000303 },
> + { 0x19c9, 0x0010, 0x00000303 },
> + { 0x19cc, 0x0010, 0x00000303 },
> + { 0x19cd, 0x0010, 0x00000303 },
> + { 0x19ce, 0x0010, 0x00000303 },
> + { 0x19cf, 0x0010, 0x00000303 },
> + { 0x1c00, 0x0002, 0x00000103 },
> + { 0x1c01, 0x0000, 0x00000001 },
> + { 0x1920, 0x0000, 0x00000103 },
> + { 0x1923, 0x0000, 0x00000103 },
> + { 0x1924, 0x0000, 0x00000103 },
> + { 0x1940, 0x0000, 0x00000103 },
> + { 0x1941, 0x0004, 0x00000103 },
> + { 0x1943, 0x0000, 0x00000103 },
> + { 0x1944, 0x0000, 0x00000103 },
> + { 0x1947, 0x0000, 0x00000103 },
> +};
This is nearly impossible to handle.
Please add defines for 0x1, 0x103 and 0x303. Also please consider
adding comments for the devices.
> +
> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> {
> return container_of(smmu, struct qcom_smmu, smmu);
> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
> .tlb_sync = qcom_smmu_tlb_sync,
> };
>
> +
> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> + .init_context = qcom_smmu_init_context,
> + .cfg_probe = qcom_smmu_cfg_probe,
> + .def_domain_type = qcom_smmu_def_domain_type,
> + .reset = arm_mmu500_reset,
> + .write_s2cr = qcom_smmu_write_s2cr,
> + .tlb_sync = qcom_smmu_tlb_sync,
> +};
> +
> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> .init_context = qcom_adreno_smmu_init_context,
> .def_domain_type = qcom_smmu_def_domain_type,
> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
> .reg_offset = qcom_smmu_impl0_reg_offset,
> };
>
> +static const struct actlr_config sm8550_actlrcfg = {
> + .adata = sm8550_apps_actlr_data,
> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> +};
> +
> /*
> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
> * there are not enough context banks.
> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
> /* Also no debug configuration. */
> };
>
> +
> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> + .impl = &sm8550_smmu_500_impl,
> + .adreno_impl = &qcom_adreno_smmu_500_impl,
> + .cfg = &qcom_smmu_impl0_cfg,
> + .actlrcfg = &sm8550_actlrcfg,
> +};
This structure doesn't seem to be linked. Did you test your patches?
> +
> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
> .impl = &qcom_smmu_500_impl,
> .adreno_impl = &qcom_adreno_smmu_500_impl,
> .cfg = &qcom_smmu_impl0_cfg,
> };
>
> -/*
> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> - * special handling and can not be covered by the qcom,smmu-500 entry.
> - */
Leave the comment in place.
> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> --
> 2.17.1
>
--
With best wishes
Dmitry
On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
<[email protected]> wrote:
>
> Currently in Qualcomm SoCs the default prefetch is set to 1 which allows
> the TLB to fetch just the next page table. MMU-500 features ACTLR
> register which is implementation defined and is used for Qualcomm SoCs
> to have a prefetch setting of 1/3/7/15 enabling TLB to prefetch
> the next set of page tables accordingly allowing for faster translations.
>
> ACTLR value is unique for each SMR (Stream matching register) and stored
> in a pre-populated table. This value is set to the register during
> context bank initialisation.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 34 ++++++++++++++++++++++
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 ++
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 ++--
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++++
> 4 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index ae7cae015193..68c1f4908473 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -14,6 +14,17 @@
>
> #define QCOM_DUMMY_VAL -1
>
> +struct actlr_config {
> + const struct actlr_data *adata;
> + u32 size;
This should be size_t.
Also could you please drop the separate struct actlr_config and move
these two fields into struct qcom_smmu_config.
> +};
> +
> +struct actlr_data {
> + u16 sid;
> + u16 mask;
> + u32 actlr;
> +};
> +
> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> {
> return container_of(smmu, struct qcom_smmu, smmu);
> @@ -270,6 +281,26 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
> static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
> {
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> + const struct actlr_config *actlrcfg;
> + struct arm_smmu_smr *smr = smmu->smrs;
> + int idx = smmu_domain->cfg.cbndx;
> + int i;
> + u16 id;
> + u16 mask;
> +
> + if (qsmmu->actlrcfg) {
> + actlrcfg = qsmmu->actlrcfg;
> + for (i = 0; i < actlrcfg->size; ++i) {
> + id = actlrcfg->adata[i].sid;
> + mask = actlrcfg->adata[i].mask;
> + if (!smr_is_subset(*smr, id, mask))
> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR,
> + actlrcfg->adata[i].actlr);
> + }
> + }
Consider extracting this to a separate function. This way you can
reduce 4 indentation levels into a single loop.
> +
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> return 0;
> @@ -459,6 +490,9 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
> qsmmu->smmu.impl = impl;
> qsmmu->cfg = data->cfg;
>
> + if (data->actlrcfg && (data->actlrcfg->size))
> + qsmmu->actlrcfg = data->actlrcfg;
> +
> return &qsmmu->smmu;
> }
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> index 593910567b88..4b6862715070 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> @@ -9,6 +9,7 @@
> struct qcom_smmu {
> struct arm_smmu_device smmu;
> const struct qcom_smmu_config *cfg;
> + const struct actlr_config *actlrcfg;
> bool bypass_quirk;
> u8 bypass_cbndx;
> u32 stall_enabled;
> @@ -25,6 +26,7 @@ struct qcom_smmu_config {
> };
>
> struct qcom_smmu_match_data {
> + const struct actlr_config *actlrcfg;
> const struct qcom_smmu_config *cfg;
> const struct arm_smmu_impl *impl;
> const struct arm_smmu_impl *adreno_impl;
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 4c79ef6f4c75..38ac1cbc799b 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -992,9 +992,10 @@ static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask)
> * expect simply identical entries for this case, but there's
> * no harm in accommodating the generalisation.
> */
> - if ((mask & smrs[i].mask) == mask &&
> - !((id ^ smrs[i].id) & ~smrs[i].mask))
> +
> + if (smr_is_subset(smrs[i], id, mask))
> return i;
> +
> /*
> * If the new entry has any other overlap with an existing one,
> * though, then there always exists at least one stream ID
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 703fd5817ec1..b1638bbc41d4 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -501,6 +501,11 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
> writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
> }
>
> +static inline bool smr_is_subset(struct arm_smmu_smr smrs, u16 id, u16 mask)
> +{
> + return (mask & smrs.mask) == mask && !((id ^ smrs.id) & ~smrs.mask);
> +}
> +
> #define ARM_SMMU_GR0 0
> #define ARM_SMMU_GR1 1
> #define ARM_SMMU_CB(s, n) ((s)->numpage + (n))
> --
> 2.17.1
>
--
With best wishes
Dmitry
On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
>> through SoC specific reset ops, which is disabled in the default MMU-500
>> reset ops, but is expected for context banks using ACTLR register to
>> retain the prefetch value during reset and runtime suspend.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 590b7c285299..f342b4778cf1 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
>> return match ? IOMMU_DOMAIN_IDENTITY : 0;
>> }
>>
>> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
>> +
>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
>> +{
>> + int i;
>> + u32 reg;
>> +
>> + arm_mmu500_reset(smmu);
>> +
>> + for (i = 0; i < smmu->num_context_banks; ++i) {
>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
>> + reg |= ARM_MMU500_ACTLR_CPRE;
>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg);
>> + }
>
> Wrong indentation. Did you run your patches through checkpatch.pl?
>
Yes Dmitry, I did run checkpatch.pl script on this patch as well as
others, got 0 errors and 0 warnings. With -f option as well. Did not
get any related errors and warnings.
>> +
>> + return 0;
>> +}
>> +
>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
>> {
>> int ret;
>>
>> - arm_mmu500_reset(smmu);
>> + qcom_smmu500_reset(smmu);
>>
>> /*
>> * To address performance degradation in non-real time clients,
>> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = {
>> .init_context = qcom_smmu_init_context,
>> .cfg_probe = qcom_smmu_cfg_probe,
>> .def_domain_type = qcom_smmu_def_domain_type,
>> - .reset = arm_mmu500_reset,
>> + .reset = qcom_smmu500_reset,
>> .write_s2cr = qcom_smmu_write_s2cr,
>> .tlb_sync = qcom_smmu_tlb_sync,
>> };
>> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>> .init_context = qcom_smmu_init_context,
>> .cfg_probe = qcom_smmu_cfg_probe,
>> .def_domain_type = qcom_smmu_def_domain_type,
>> - .reset = arm_mmu500_reset,
>> + .reset = qcom_smmu500_reset,
>> .write_s2cr = qcom_smmu_write_s2cr,
>> .tlb_sync = qcom_smmu_tlb_sync,
>> };
>> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = {
>> .init_context = qcom_adreno_smmu_init_context,
>> .def_domain_type = qcom_smmu_def_domain_type,
>> - .reset = arm_mmu500_reset,
>> + .reset = qcom_smmu500_reset,
>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
>> .write_sctlr = qcom_adreno_smmu_write_sctlr,
>> .tlb_sync = qcom_smmu_tlb_sync,
>> --
>> 2.17.1
>>
>
>
On Sat, 4 Nov 2023 at 00:07, Bibek Kumar Patro
<[email protected]> wrote:
>
>
>
> On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote:
> > On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> > <[email protected]> wrote:
> >>
> >> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
> >> through SoC specific reset ops, which is disabled in the default MMU-500
> >> reset ops, but is expected for context banks using ACTLR register to
> >> retain the prefetch value during reset and runtime suspend.
> >>
> >> Signed-off-by: Bibek Kumar Patro <[email protected]>
> >> ---
> >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
> >> 1 file changed, 22 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> index 590b7c285299..f342b4778cf1 100644
> >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
> >> return match ? IOMMU_DOMAIN_IDENTITY : 0;
> >> }
> >>
> >> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
> >> +
> >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
> >> +{
> >> + int i;
> >> + u32 reg;
> >> +
> >> + arm_mmu500_reset(smmu);
> >> +
> >> + for (i = 0; i < smmu->num_context_banks; ++i) {
> >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
> >> + reg |= ARM_MMU500_ACTLR_CPRE;
> >> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg);
> >> + }
> >
> > Wrong indentation. Did you run your patches through checkpatch.pl?
> >
>
> Yes Dmitry, I did run checkpatch.pl script on this patch as well as
> others, got 0 errors and 0 warnings. With -f option as well. Did not
> get any related errors and warnings.
Ack, I beg your pardon. checkpatch indeed doesn't warn about this
indentation. Though it is still incorrect.
>
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
> >> {
> >> int ret;
> >>
> >> - arm_mmu500_reset(smmu);
> >> + qcom_smmu500_reset(smmu);
> >>
> >> /*
> >> * To address performance degradation in non-real time clients,
> >> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = {
> >> .init_context = qcom_smmu_init_context,
> >> .cfg_probe = qcom_smmu_cfg_probe,
> >> .def_domain_type = qcom_smmu_def_domain_type,
> >> - .reset = arm_mmu500_reset,
> >> + .reset = qcom_smmu500_reset,
> >> .write_s2cr = qcom_smmu_write_s2cr,
> >> .tlb_sync = qcom_smmu_tlb_sync,
> >> };
> >> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> >> .init_context = qcom_smmu_init_context,
> >> .cfg_probe = qcom_smmu_cfg_probe,
> >> .def_domain_type = qcom_smmu_def_domain_type,
> >> - .reset = arm_mmu500_reset,
> >> + .reset = qcom_smmu500_reset,
> >> .write_s2cr = qcom_smmu_write_s2cr,
> >> .tlb_sync = qcom_smmu_tlb_sync,
> >> };
> >> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> >> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = {
> >> .init_context = qcom_adreno_smmu_init_context,
> >> .def_domain_type = qcom_smmu_def_domain_type,
> >> - .reset = arm_mmu500_reset,
> >> + .reset = qcom_smmu500_reset,
> >> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
> >> .write_sctlr = qcom_adreno_smmu_write_sctlr,
> >> .tlb_sync = qcom_smmu_tlb_sync,
> >> --
> >> 2.17.1
> >>
> >
> >
--
With best wishes
Dmitry
On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>> Add ACTLR data table for SM8550 along with support for
>> same including SM8550 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>> 1 file changed, 81 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 68c1f4908473..590b7c285299 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -25,6 +25,64 @@ struct actlr_data {
>> u32 actlr;
>> };
>>
>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>> + { 0x18a0, 0x0000, 0x00000103 },
>> + { 0x18e0, 0x0000, 0x00000103 },
>> + { 0x0800, 0x0020, 0x00000001 },
>> + { 0x1800, 0x00c0, 0x00000001 },
>> + { 0x1820, 0x0000, 0x00000001 },
>> + { 0x1860, 0x0000, 0x00000001 },
>> + { 0x0c01, 0x0020, 0x00000303 },
>> + { 0x0c02, 0x0020, 0x00000303 },
>> + { 0x0c03, 0x0020, 0x00000303 },
>> + { 0x0c04, 0x0020, 0x00000303 },
>> + { 0x0c05, 0x0020, 0x00000303 },
>> + { 0x0c06, 0x0020, 0x00000303 },
>> + { 0x0c07, 0x0020, 0x00000303 },
>> + { 0x0c08, 0x0020, 0x00000303 },
>> + { 0x0c09, 0x0020, 0x00000303 },
>> + { 0x0c0c, 0x0020, 0x00000303 },
>> + { 0x0c0d, 0x0020, 0x00000303 },
>> + { 0x0c0e, 0x0020, 0x00000303 },
>> + { 0x0c0f, 0x0020, 0x00000303 },
>> + { 0x1961, 0x0000, 0x00000303 },
>> + { 0x1962, 0x0000, 0x00000303 },
>> + { 0x1963, 0x0000, 0x00000303 },
>> + { 0x1964, 0x0000, 0x00000303 },
>> + { 0x1965, 0x0000, 0x00000303 },
>> + { 0x1966, 0x0000, 0x00000303 },
>> + { 0x1967, 0x0000, 0x00000303 },
>> + { 0x1968, 0x0000, 0x00000303 },
>> + { 0x1969, 0x0000, 0x00000303 },
>> + { 0x196c, 0x0000, 0x00000303 },
>> + { 0x196d, 0x0000, 0x00000303 },
>> + { 0x196e, 0x0000, 0x00000303 },
>> + { 0x196f, 0x0000, 0x00000303 },
>> + { 0x19c1, 0x0010, 0x00000303 },
>> + { 0x19c2, 0x0010, 0x00000303 },
>> + { 0x19c3, 0x0010, 0x00000303 },
>> + { 0x19c4, 0x0010, 0x00000303 },
>> + { 0x19c5, 0x0010, 0x00000303 },
>> + { 0x19c6, 0x0010, 0x00000303 },
>> + { 0x19c7, 0x0010, 0x00000303 },
>> + { 0x19c8, 0x0010, 0x00000303 },
>> + { 0x19c9, 0x0010, 0x00000303 },
>> + { 0x19cc, 0x0010, 0x00000303 },
>> + { 0x19cd, 0x0010, 0x00000303 },
>> + { 0x19ce, 0x0010, 0x00000303 },
>> + { 0x19cf, 0x0010, 0x00000303 },
>> + { 0x1c00, 0x0002, 0x00000103 },
>> + { 0x1c01, 0x0000, 0x00000001 },
>> + { 0x1920, 0x0000, 0x00000103 },
>> + { 0x1923, 0x0000, 0x00000103 },
>> + { 0x1924, 0x0000, 0x00000103 },
>> + { 0x1940, 0x0000, 0x00000103 },
>> + { 0x1941, 0x0004, 0x00000103 },
>> + { 0x1943, 0x0000, 0x00000103 },
>> + { 0x1944, 0x0000, 0x00000103 },
>> + { 0x1947, 0x0000, 0x00000103 },
>> +};
>
> This is nearly impossible to handle.
> Please add defines for 0x1, 0x103 and 0x303. Also please consider
> adding comments for the devices.
>
Ack, Initially added the comments for devices, but since only driver is
handling this data, and clients won't refer this so removed it. Will
consider adding it back. This actlr field value might different (other
than 0x1,0x103,0x303) in other platforms as per my anticipation,
depending on the bit settings, so won't the defines change with
different platforms? Hence this register setting value might be apt?
>> +
>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>> {
>> return container_of(smmu, struct qcom_smmu, smmu);
>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>> .tlb_sync = qcom_smmu_tlb_sync,
>> };
>>
>> +
>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>> + .init_context = qcom_smmu_init_context,
>> + .cfg_probe = qcom_smmu_cfg_probe,
>> + .def_domain_type = qcom_smmu_def_domain_type,
>> + .reset = arm_mmu500_reset,
>> + .write_s2cr = qcom_smmu_write_s2cr,
>> + .tlb_sync = qcom_smmu_tlb_sync,
>> +};
>> +
>> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>> .init_context = qcom_adreno_smmu_init_context,
>> .def_domain_type = qcom_smmu_def_domain_type,
>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>> .reg_offset = qcom_smmu_impl0_reg_offset,
>> };
>>
>> +static const struct actlr_config sm8550_actlrcfg = {
>> + .adata = sm8550_apps_actlr_data,
>> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>> +};
>> +
>> /*
>> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>> * there are not enough context banks.
>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>> /* Also no debug configuration. */
>> };
>>
>> +
>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>> + .impl = &sm8550_smmu_500_impl,
>> + .adreno_impl = &qcom_adreno_smmu_500_impl,
>> + .cfg = &qcom_smmu_impl0_cfg,
>> + .actlrcfg = &sm8550_actlrcfg,
>> +};
>
> This structure doesn't seem to be linked. Did you test your patches?
>
Apologies Dmitry, just checked back my patches, I tested it but while
refining the patches I somehow missed this link
{ .compatible = "qcom,sm8550-smmu-500", .data =
&sm8550_smmu_500_impl0_data };
in below qcom_smmu_500_impl0_data structure.
I will take care of this in next version.
>> +
>> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>> .impl = &qcom_smmu_500_impl,
>> .adreno_impl = &qcom_adreno_smmu_500_impl,
>> .cfg = &qcom_smmu_impl0_cfg,
>> };
>>
>> -/*
>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>> - */
>
> Leave the comment in place.
>
Thanks for this comment which helped me to note the above mentioned
mistake.
>> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>> --
>> 2.17.1
>>
>
>
On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
<[email protected]> wrote:
>
>
>
> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> > On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> > <[email protected]> wrote:
> >>
> >> Add ACTLR data table for SM8550 along with support for
> >> same including SM8550 specific implementation operations.
> >>
> >> Signed-off-by: Bibek Kumar Patro <[email protected]>
> >> ---
> >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
> >> 1 file changed, 81 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> index 68c1f4908473..590b7c285299 100644
> >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> @@ -25,6 +25,64 @@ struct actlr_data {
> >> u32 actlr;
> >> };
> >>
> >> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> >> + { 0x18a0, 0x0000, 0x00000103 },
> >> + { 0x18e0, 0x0000, 0x00000103 },
> >> + { 0x0800, 0x0020, 0x00000001 },
> >> + { 0x1800, 0x00c0, 0x00000001 },
> >> + { 0x1820, 0x0000, 0x00000001 },
> >> + { 0x1860, 0x0000, 0x00000001 },
> >> + { 0x0c01, 0x0020, 0x00000303 },
> >> + { 0x0c02, 0x0020, 0x00000303 },
> >> + { 0x0c03, 0x0020, 0x00000303 },
> >> + { 0x0c04, 0x0020, 0x00000303 },
> >> + { 0x0c05, 0x0020, 0x00000303 },
> >> + { 0x0c06, 0x0020, 0x00000303 },
> >> + { 0x0c07, 0x0020, 0x00000303 },
> >> + { 0x0c08, 0x0020, 0x00000303 },
> >> + { 0x0c09, 0x0020, 0x00000303 },
> >> + { 0x0c0c, 0x0020, 0x00000303 },
> >> + { 0x0c0d, 0x0020, 0x00000303 },
> >> + { 0x0c0e, 0x0020, 0x00000303 },
> >> + { 0x0c0f, 0x0020, 0x00000303 },
> >> + { 0x1961, 0x0000, 0x00000303 },
> >> + { 0x1962, 0x0000, 0x00000303 },
> >> + { 0x1963, 0x0000, 0x00000303 },
> >> + { 0x1964, 0x0000, 0x00000303 },
> >> + { 0x1965, 0x0000, 0x00000303 },
> >> + { 0x1966, 0x0000, 0x00000303 },
> >> + { 0x1967, 0x0000, 0x00000303 },
> >> + { 0x1968, 0x0000, 0x00000303 },
> >> + { 0x1969, 0x0000, 0x00000303 },
> >> + { 0x196c, 0x0000, 0x00000303 },
> >> + { 0x196d, 0x0000, 0x00000303 },
> >> + { 0x196e, 0x0000, 0x00000303 },
> >> + { 0x196f, 0x0000, 0x00000303 },
> >> + { 0x19c1, 0x0010, 0x00000303 },
> >> + { 0x19c2, 0x0010, 0x00000303 },
> >> + { 0x19c3, 0x0010, 0x00000303 },
> >> + { 0x19c4, 0x0010, 0x00000303 },
> >> + { 0x19c5, 0x0010, 0x00000303 },
> >> + { 0x19c6, 0x0010, 0x00000303 },
> >> + { 0x19c7, 0x0010, 0x00000303 },
> >> + { 0x19c8, 0x0010, 0x00000303 },
> >> + { 0x19c9, 0x0010, 0x00000303 },
> >> + { 0x19cc, 0x0010, 0x00000303 },
> >> + { 0x19cd, 0x0010, 0x00000303 },
> >> + { 0x19ce, 0x0010, 0x00000303 },
> >> + { 0x19cf, 0x0010, 0x00000303 },
> >> + { 0x1c00, 0x0002, 0x00000103 },
> >> + { 0x1c01, 0x0000, 0x00000001 },
> >> + { 0x1920, 0x0000, 0x00000103 },
> >> + { 0x1923, 0x0000, 0x00000103 },
> >> + { 0x1924, 0x0000, 0x00000103 },
> >> + { 0x1940, 0x0000, 0x00000103 },
> >> + { 0x1941, 0x0004, 0x00000103 },
> >> + { 0x1943, 0x0000, 0x00000103 },
> >> + { 0x1944, 0x0000, 0x00000103 },
> >> + { 0x1947, 0x0000, 0x00000103 },
> >> +};
> >
> > This is nearly impossible to handle.
> > Please add defines for 0x1, 0x103 and 0x303. Also please consider
> > adding comments for the devices.
> >
>
> Ack, Initially added the comments for devices, but since only driver is
> handling this data, and clients won't refer this so removed it. Will
> consider adding it back.
It will help developers / porters who will try matching the SID and the device.
> This actlr field value might different (other
> than 0x1,0x103,0x303) in other platforms as per my anticipation,
> depending on the bit settings, so won't the defines change with
> different platforms? Hence this register setting value might be apt?
It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
provide sensible defines so that we can understand and review them.
Other platforms might use new defines.
>
> >> +
> >> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> >> {
> >> return container_of(smmu, struct qcom_smmu, smmu);
> >> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
> >> .tlb_sync = qcom_smmu_tlb_sync,
> >> };
> >>
> >> +
> >> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> >> + .init_context = qcom_smmu_init_context,
> >> + .cfg_probe = qcom_smmu_cfg_probe,
> >> + .def_domain_type = qcom_smmu_def_domain_type,
> >> + .reset = arm_mmu500_reset,
> >> + .write_s2cr = qcom_smmu_write_s2cr,
> >> + .tlb_sync = qcom_smmu_tlb_sync,
> >> +};
> >> +
> >> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> >> .init_context = qcom_adreno_smmu_init_context,
> >> .def_domain_type = qcom_smmu_def_domain_type,
> >> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
> >> .reg_offset = qcom_smmu_impl0_reg_offset,
> >> };
> >>
> >> +static const struct actlr_config sm8550_actlrcfg = {
> >> + .adata = sm8550_apps_actlr_data,
> >> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> >> +};
> >> +
> >> /*
> >> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
> >> * there are not enough context banks.
> >> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
> >> /* Also no debug configuration. */
> >> };
> >>
> >> +
> >> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> >> + .impl = &sm8550_smmu_500_impl,
> >> + .adreno_impl = &qcom_adreno_smmu_500_impl,
> >> + .cfg = &qcom_smmu_impl0_cfg,
> >> + .actlrcfg = &sm8550_actlrcfg,
> >> +};
> >
> > This structure doesn't seem to be linked. Did you test your patches?
> >
>
> Apologies Dmitry, just checked back my patches, I tested it but while
> refining the patches I somehow missed this link
> { .compatible = "qcom,sm8550-smmu-500", .data =
> &sm8550_smmu_500_impl0_data };
> in below qcom_smmu_500_impl0_data structure.
> I will take care of this in next version.
>
> >> +
> >> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
> >> .impl = &qcom_smmu_500_impl,
> >> .adreno_impl = &qcom_adreno_smmu_500_impl,
> >> .cfg = &qcom_smmu_impl0_cfg,
> >> };
> >>
> >> -/*
> >> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> >> - * special handling and can not be covered by the qcom,smmu-500 entry.
> >> - */
> >
> > Leave the comment in place.
> >
>
> Thanks for this comment which helped me to note the above mentioned
> mistake.
>
> >> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> >> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
> >> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> >> --
> >> 2.17.1
> >>
> >
> >
--
With best wishes
Dmitry
On 11/4/2023 3:53 AM, Dmitry Baryshkov wrote:
> On Sat, 4 Nov 2023 at 00:07, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>>
>>
>> On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote:
>>> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
>>> <[email protected]> wrote:
>>>>
>>>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
>>>> through SoC specific reset ops, which is disabled in the default MMU-500
>>>> reset ops, but is expected for context banks using ACTLR register to
>>>> retain the prefetch value during reset and runtime suspend.
>>>>
>>>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>>>> ---
>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
>>>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> index 590b7c285299..f342b4778cf1 100644
>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
>>>> return match ? IOMMU_DOMAIN_IDENTITY : 0;
>>>> }
>>>>
>>>> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
>>>> +
>>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
>>>> +{
>>>> + int i;
>>>> + u32 reg;
>>>> +
>>>> + arm_mmu500_reset(smmu);
>>>> +
>>>> + for (i = 0; i < smmu->num_context_banks; ++i) {
>>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
>>>> + reg |= ARM_MMU500_ACTLR_CPRE;
>>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg);
>>>> + }
>>>
>>> Wrong indentation. Did you run your patches through checkpatch.pl?
>>>
>>
>> Yes Dmitry, I did run checkpatch.pl script on this patch as well as
>> others, got 0 errors and 0 warnings. With -f option as well. Did not
>> get any related errors and warnings.
>
> Ack, I beg your pardon. checkpatch indeed doesn't warn about this
> indentation. Though it is still incorrect.
>
Ack, thanks for pointing this out. Will fix this indentation for loop in
next iteration.
>>
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu)
>>>> {
>>>> int ret;
>>>>
>>>> - arm_mmu500_reset(smmu);
>>>> + qcom_smmu500_reset(smmu);
>>>>
>>>> /*
>>>> * To address performance degradation in non-real time clients,
>>>> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = {
>>>> .init_context = qcom_smmu_init_context,
>>>> .cfg_probe = qcom_smmu_cfg_probe,
>>>> .def_domain_type = qcom_smmu_def_domain_type,
>>>> - .reset = arm_mmu500_reset,
>>>> + .reset = qcom_smmu500_reset,
>>>> .write_s2cr = qcom_smmu_write_s2cr,
>>>> .tlb_sync = qcom_smmu_tlb_sync,
>>>> };
>>>> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>> .init_context = qcom_smmu_init_context,
>>>> .cfg_probe = qcom_smmu_cfg_probe,
>>>> .def_domain_type = qcom_smmu_def_domain_type,
>>>> - .reset = arm_mmu500_reset,
>>>> + .reset = qcom_smmu500_reset,
>>>> .write_s2cr = qcom_smmu_write_s2cr,
>>>> .tlb_sync = qcom_smmu_tlb_sync,
>>>> };
>>>> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = {
>>>> .init_context = qcom_adreno_smmu_init_context,
>>>> .def_domain_type = qcom_smmu_def_domain_type,
>>>> - .reset = arm_mmu500_reset,
>>>> + .reset = qcom_smmu500_reset,
>>>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank,
>>>> .write_sctlr = qcom_adreno_smmu_write_sctlr,
>>>> .tlb_sync = qcom_smmu_tlb_sync,
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
>
>
>
On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
<[email protected]> wrote:
>
> This patch series consists of three parts and covers the following:
>
> 1. Introducing intital set of driver changes to implement ACTLR register
> for custom prefetcher settings in Qualcomm SoCs.
>
> 2. Adding ACTLR data and implementation operations for SM8550.
>
> 3. Re-enabling context caching for Qualcomm SoCs to retain prefetcher
> settings during reset and runtime suspend.
>
> Link to discussion on RFC posted on ACTLR implementation:
> https://lore.kernel.org/all/[email protected]/
First two patches in that thread were not archived for some reason.
However, as there was a previous iteration, it is suggested to include
the changelog between RFC and v1.
>
> Bibek Kumar Patro (3):
> iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings
> iommu/arm-smmu: add ACTLR data and support for sm8550
> iommu/arm-smmu: re-enable context caching in smmu reset operation
>
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 143 ++++++++++++++++++++-
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 +
Although it is not mentioned in the MAINTAINERS file, could you please
CC linux-arm-msm when sending v2?
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 +
> 4 files changed, 146 insertions(+), 9 deletions(-)
>
> --
> 2.17.1
>
--
With best wishes
Dmitry
On 11/3/23 22:51, Bibek Kumar Patro wrote:
> Add ACTLR data table for SM8550 along with support for
> same including SM8550 specific implementation operations.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
> 1 file changed, 81 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 68c1f4908473..590b7c285299 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -25,6 +25,64 @@ struct actlr_data {
> u32 actlr;
> };
>
> +static const struct actlr_data sm8550_apps_actlr_data[] = {
I assume this data will be different for each SoC.. perhaps
moving this to a separate file (not sure if dt makes sense if
it's hardcoded per platform) makes sense.
This will also assume that these can not differ between firmware
versions.
Konrad
On 11/3/23 22:51, Bibek Kumar Patro wrote:
> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
> through SoC specific reset ops, which is disabled in the default MMU-500
> reset ops, but is expected for context banks using ACTLR register to
> retain the prefetch value during reset and runtime suspend.
>
> Signed-off-by: Bibek Kumar Patro <[email protected]>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 590b7c285299..f342b4778cf1 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev)
> return match ? IOMMU_DOMAIN_IDENTITY : 0;
> }
>
> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
> +
> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
> +{
> + int i;
> + u32 reg;
> +
> + arm_mmu500_reset(smmu);
> +
> + for (i = 0; i < smmu->num_context_banks; ++i) {
This loop deserves a comment above it like
/* Re-enable context caching after reset */
Konrad
Hi Bibek,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.6]
[also build test WARNING on linus/master next-20231103]
[cannot apply to joro-iommu/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bibek-Kumar-Patro/iommu-arm-smmu-introduction-of-ACTLR-for-custom-prefetcher-settings/20231104-055625
base: v6.6
patch link: https://lore.kernel.org/r/20231103215124.1095-3-quic_bibekkum%40quicinc.com
patch subject: [PATCH 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231104/[email protected]/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:614:42: warning: 'sm8550_smmu_500_impl0_data' defined but not used [-Wunused-const-variable=]
614 | static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/sm8550_smmu_500_impl0_data +614 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
612
613
> 614 static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
615 .impl = &sm8550_smmu_500_impl,
616 .adreno_impl = &qcom_adreno_smmu_500_impl,
617 .cfg = &qcom_smmu_impl0_cfg,
618 .actlrcfg = &sm8550_actlrcfg,
619 };
620
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
> On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>>
>>
>> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
>>> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
>>> <[email protected]> wrote:
>>>>
>>>> Add ACTLR data table for SM8550 along with support for
>>>> same including SM8550 specific implementation operations.
>>>>
>>>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>>>> ---
>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>>>> 1 file changed, 81 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> index 68c1f4908473..590b7c285299 100644
>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> @@ -25,6 +25,64 @@ struct actlr_data {
>>>> u32 actlr;
>>>> };
>>>>
>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>>>> + { 0x18a0, 0x0000, 0x00000103 },
>>>> + { 0x18e0, 0x0000, 0x00000103 },
>>>> + { 0x0800, 0x0020, 0x00000001 },
>>>> + { 0x1800, 0x00c0, 0x00000001 },
>>>> + { 0x1820, 0x0000, 0x00000001 },
>>>> + { 0x1860, 0x0000, 0x00000001 },
>>>> + { 0x0c01, 0x0020, 0x00000303 },
>>>> + { 0x0c02, 0x0020, 0x00000303 },
>>>> + { 0x0c03, 0x0020, 0x00000303 },
>>>> + { 0x0c04, 0x0020, 0x00000303 },
>>>> + { 0x0c05, 0x0020, 0x00000303 },
>>>> + { 0x0c06, 0x0020, 0x00000303 },
>>>> + { 0x0c07, 0x0020, 0x00000303 },
>>>> + { 0x0c08, 0x0020, 0x00000303 },
>>>> + { 0x0c09, 0x0020, 0x00000303 },
>>>> + { 0x0c0c, 0x0020, 0x00000303 },
>>>> + { 0x0c0d, 0x0020, 0x00000303 },
>>>> + { 0x0c0e, 0x0020, 0x00000303 },
>>>> + { 0x0c0f, 0x0020, 0x00000303 },
>>>> + { 0x1961, 0x0000, 0x00000303 },
>>>> + { 0x1962, 0x0000, 0x00000303 },
>>>> + { 0x1963, 0x0000, 0x00000303 },
>>>> + { 0x1964, 0x0000, 0x00000303 },
>>>> + { 0x1965, 0x0000, 0x00000303 },
>>>> + { 0x1966, 0x0000, 0x00000303 },
>>>> + { 0x1967, 0x0000, 0x00000303 },
>>>> + { 0x1968, 0x0000, 0x00000303 },
>>>> + { 0x1969, 0x0000, 0x00000303 },
>>>> + { 0x196c, 0x0000, 0x00000303 },
>>>> + { 0x196d, 0x0000, 0x00000303 },
>>>> + { 0x196e, 0x0000, 0x00000303 },
>>>> + { 0x196f, 0x0000, 0x00000303 },
>>>> + { 0x19c1, 0x0010, 0x00000303 },
>>>> + { 0x19c2, 0x0010, 0x00000303 },
>>>> + { 0x19c3, 0x0010, 0x00000303 },
>>>> + { 0x19c4, 0x0010, 0x00000303 },
>>>> + { 0x19c5, 0x0010, 0x00000303 },
>>>> + { 0x19c6, 0x0010, 0x00000303 },
>>>> + { 0x19c7, 0x0010, 0x00000303 },
>>>> + { 0x19c8, 0x0010, 0x00000303 },
>>>> + { 0x19c9, 0x0010, 0x00000303 },
>>>> + { 0x19cc, 0x0010, 0x00000303 },
>>>> + { 0x19cd, 0x0010, 0x00000303 },
>>>> + { 0x19ce, 0x0010, 0x00000303 },
>>>> + { 0x19cf, 0x0010, 0x00000303 },
>>>> + { 0x1c00, 0x0002, 0x00000103 },
>>>> + { 0x1c01, 0x0000, 0x00000001 },
>>>> + { 0x1920, 0x0000, 0x00000103 },
>>>> + { 0x1923, 0x0000, 0x00000103 },
>>>> + { 0x1924, 0x0000, 0x00000103 },
>>>> + { 0x1940, 0x0000, 0x00000103 },
>>>> + { 0x1941, 0x0004, 0x00000103 },
>>>> + { 0x1943, 0x0000, 0x00000103 },
>>>> + { 0x1944, 0x0000, 0x00000103 },
>>>> + { 0x1947, 0x0000, 0x00000103 },
>>>> +};
>>>
>>> This is nearly impossible to handle.
>>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
>>> adding comments for the devices.
>>>
>>
>> Ack, Initially added the comments for devices, but since only driver is
>> handling this data, and clients won't refer this so removed it. Will
>> consider adding it back.
>
> It will help developers / porters who will try matching the SID and the device.
>
Agree on the same, I'll add those comments for devices back.
>> This actlr field value might different (other
>> than 0x1,0x103,0x303) in other platforms as per my anticipation,
>> depending on the bit settings, so won't the defines change with
>> different platforms? Hence this register setting value might be apt?
>
> It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
> provide sensible defines so that we can understand and review them.
>
Understandable, In next patch I'll populate the actlr_data in
following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
the actlr values (0x1,0x103,0x303).
This would help in understanding these values. Hope this
proposed format will be okay?
> Other platforms might use new defines.
>
>>
>>>> +
>>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>>> {
>>>> return container_of(smmu, struct qcom_smmu, smmu);
>>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>>>> .tlb_sync = qcom_smmu_tlb_sync,
>>>> };
>>>>
>>>> +
>>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>> + .init_context = qcom_smmu_init_context,
>>>> + .cfg_probe = qcom_smmu_cfg_probe,
>>>> + .def_domain_type = qcom_smmu_def_domain_type,
>>>> + .reset = arm_mmu500_reset,
>>>> + .write_s2cr = qcom_smmu_write_s2cr,
>>>> + .tlb_sync = qcom_smmu_tlb_sync,
>>>> +};
>>>> +
>>>> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>> .init_context = qcom_adreno_smmu_init_context,
>>>> .def_domain_type = qcom_smmu_def_domain_type,
>>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>>>> .reg_offset = qcom_smmu_impl0_reg_offset,
>>>> };
>>>>
>>>> +static const struct actlr_config sm8550_actlrcfg = {
>>>> + .adata = sm8550_apps_actlr_data,
>>>> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>>>> +};
>>>> +
>>>> /*
>>>> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>>>> * there are not enough context banks.
>>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>>>> /* Also no debug configuration. */
>>>> };
>>>>
>>>> +
>>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>>>> + .impl = &sm8550_smmu_500_impl,
>>>> + .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>> + .cfg = &qcom_smmu_impl0_cfg,
>>>> + .actlrcfg = &sm8550_actlrcfg,
>>>> +};
>>>
>>> This structure doesn't seem to be linked. Did you test your patches?
>>>
>>
>> Apologies Dmitry, just checked back my patches, I tested it but while
>> refining the patches I somehow missed this link
>> { .compatible = "qcom,sm8550-smmu-500", .data =
>> &sm8550_smmu_500_impl0_data };
>> in below qcom_smmu_500_impl0_data structure.
>> I will take care of this in next version.
>>
>>>> +
>>>> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>>>> .impl = &qcom_smmu_500_impl,
>>>> .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>> .cfg = &qcom_smmu_impl0_cfg,
>>>> };
>>>>
>>>> -/*
>>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>>>> - */
>>>
>>> Leave the comment in place.
>>>
>>
>> Thanks for this comment which helped me to note the above mentioned
>> mistake.
>>
>>>> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>>>> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>>>> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
>
>
>
On 11/4/2023 3:33 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>> Currently in Qualcomm SoCs the default prefetch is set to 1 which allows
>> the TLB to fetch just the next page table. MMU-500 features ACTLR
>> register which is implementation defined and is used for Qualcomm SoCs
>> to have a prefetch setting of 1/3/7/15 enabling TLB to prefetch
>> the next set of page tables accordingly allowing for faster translations.
>>
>> ACTLR value is unique for each SMR (Stream matching register) and stored
>> in a pre-populated table. This value is set to the register during
>> context bank initialisation.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 34 ++++++++++++++++++++++
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 ++
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 ++--
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++++
>> 4 files changed, 44 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index ae7cae015193..68c1f4908473 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -14,6 +14,17 @@
>>
>> #define QCOM_DUMMY_VAL -1
>>
>> +struct actlr_config {
>> + const struct actlr_data *adata;
>> + u32 size;
>
> This should be size_t.
>
> Also could you please drop the separate struct actlr_config and move
> these two fields into struct qcom_smmu_config.
>
Ack, will address both these inputs in the next patch.
>> +};
>> +
>> +struct actlr_data {
>> + u16 sid;
>> + u16 mask;
>> + u32 actlr;
>> +};
>> +
>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>> {
>> return container_of(smmu, struct qcom_smmu, smmu);
>> @@ -270,6 +281,26 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
>> static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>> {
>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> + const struct actlr_config *actlrcfg;
>> + struct arm_smmu_smr *smr = smmu->smrs;
>> + int idx = smmu_domain->cfg.cbndx;
>> + int i;
>> + u16 id;
>> + u16 mask;
>> +
>> + if (qsmmu->actlrcfg) {
>> + actlrcfg = qsmmu->actlrcfg;
>> + for (i = 0; i < actlrcfg->size; ++i) {
>> + id = actlrcfg->adata[i].sid;
>> + mask = actlrcfg->adata[i].mask;
>> + if (!smr_is_subset(*smr, id, mask))
>> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR,
>> + actlrcfg->adata[i].actlr);
>> + }
>> + }
>
> Consider extracting this to a separate function. This way you can
> reduce 4 indentation levels into a single loop.
>
Ack, thanks for this sugestion. Will move this entire for loop into a
separate function for simplicity reduced indent levels.
>> +
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> return 0;
>> @@ -459,6 +490,9 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
>> qsmmu->smmu.impl = impl;
>> qsmmu->cfg = data->cfg;
>>
>> + if (data->actlrcfg && (data->actlrcfg->size))
>> + qsmmu->actlrcfg = data->actlrcfg;
>> +
>> return &qsmmu->smmu;
>> }
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> index 593910567b88..4b6862715070 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> @@ -9,6 +9,7 @@
>> struct qcom_smmu {
>> struct arm_smmu_device smmu;
>> const struct qcom_smmu_config *cfg;
>> + const struct actlr_config *actlrcfg;
>> bool bypass_quirk;
>> u8 bypass_cbndx;
>> u32 stall_enabled;
>> @@ -25,6 +26,7 @@ struct qcom_smmu_config {
>> };
>>
>> struct qcom_smmu_match_data {
>> + const struct actlr_config *actlrcfg;
>> const struct qcom_smmu_config *cfg;
>> const struct arm_smmu_impl *impl;
>> const struct arm_smmu_impl *adreno_impl;
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> index 4c79ef6f4c75..38ac1cbc799b 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -992,9 +992,10 @@ static int arm_smmu_find_sme(struct arm_smmu_device *smmu, u16 id, u16 mask)
>> * expect simply identical entries for this case, but there's
>> * no harm in accommodating the generalisation.
>> */
>> - if ((mask & smrs[i].mask) == mask &&
>> - !((id ^ smrs[i].id) & ~smrs[i].mask))
>> +
>> + if (smr_is_subset(smrs[i], id, mask))
>> return i;
>> +
>> /*
>> * If the new entry has any other overlap with an existing one,
>> * though, then there always exists at least one stream ID
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
>> index 703fd5817ec1..b1638bbc41d4 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
>> @@ -501,6 +501,11 @@ static inline void arm_smmu_writeq(struct arm_smmu_device *smmu, int page,
>> writeq_relaxed(val, arm_smmu_page(smmu, page) + offset);
>> }
>>
>> +static inline bool smr_is_subset(struct arm_smmu_smr smrs, u16 id, u16 mask)
>> +{
>> + return (mask & smrs.mask) == mask && !((id ^ smrs.id) & ~smrs.mask);
>> +}
>> +
>> #define ARM_SMMU_GR0 0
>> #define ARM_SMMU_GR1 1
>> #define ARM_SMMU_CB(s, n) ((s)->numpage + (n))
>> --
>> 2.17.1
>>
>
>
On 11/4/2023 4:59 PM, Konrad Dybcio wrote:
>
>
> On 11/3/23 22:51, Bibek Kumar Patro wrote:
>> Add ACTLR data table for SM8550 along with support for
>> same including SM8550 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>> 1 file changed, 81 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 68c1f4908473..590b7c285299 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -25,6 +25,64 @@ struct actlr_data {
>> u32 actlr;
>> };
>>
>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> I assume this data will be different for each SoC.. perhaps
> moving this to a separate file (not sure if dt makes sense if
> it's hardcoded per platform) makes sense.
>
Yes, this data will be different for each SoC.
Right, adding these properties in dt won't be a right thing
since this is a register setting and not a hardware defining property.
As per my understanding passing register content via device tree is
highly discouraged, so hosting this data inside the driver.
For reference, adding the RFC link archiving this discussion:
https://lore.kernel.org/all/[email protected]/#t
If my recollection is correct, some drivers like llcc is also
following similar implementation
drivers/phy/qualcomm/phy-qcom-qmp-combo.c
drivers/soc/qcom/llcc-qcom.c
> This will also assume that these can not differ between firmware
> versions.
>
Right, these won't differ between firmware versions.
Thanks & regards,
Bibek
> Konrad
On 11/4/2023 5:00 PM, Konrad Dybcio wrote:
>
>
> On 11/3/23 22:51, Bibek Kumar Patro wrote:
>> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs
>> through SoC specific reset ops, which is disabled in the default MMU-500
>> reset ops, but is expected for context banks using ACTLR register to
>> retain the prefetch value during reset and runtime suspend.
>>
>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++----
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 590b7c285299..f342b4778cf1 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct
>> device *dev)
>> return match ? IOMMU_DOMAIN_IDENTITY : 0;
>> }
>>
>> +#define ARM_MMU500_ACTLR_CPRE BIT(1)
>> +
>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
>> +{
>> + int i;
>> + u32 reg;
>> +
>> + arm_mmu500_reset(smmu);
>> +
>> + for (i = 0; i < smmu->num_context_banks; ++i) {
> This loop deserves a comment above it like
>
> /* Re-enable context caching after reset */
>
Ack, thanks for this suggestion, this might help to explain
this addition.
I would incorporate this in next patch.
regards,
Bibek
> Konrad
On 11/4/2023 4:14 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>> This patch series consists of three parts and covers the following:
>>
>> 1. Introducing intital set of driver changes to implement ACTLR register
>> for custom prefetcher settings in Qualcomm SoCs.
>>
>> 2. Adding ACTLR data and implementation operations for SM8550.
>>
>> 3. Re-enabling context caching for Qualcomm SoCs to retain prefetcher
>> settings during reset and runtime suspend.
>>
>> Link to discussion on RFC posted on ACTLR implementation:
>> https://lore.kernel.org/all/[email protected]/
>
> First two patches in that thread were not archived for some reason.
>
> However, as there was a previous iteration, it is suggested to include
> the changelog between RFC and v1.
>
Right, in the initial RFC sent somehow only the replies were archived.
Although the RFC patch was a WIP patch in rough state, sent to take
upstream inputs.
Will try adding the changelog in v2.
To give a context, in the RFC patch, we we're initially proposing a dt
based approach to set the actlr property, where we would piggyback on
the iommus property to store actlr value and set the property during
smmu device probe.
>>
>> Bibek Kumar Patro (3):
>> iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings
>> iommu/arm-smmu: add ACTLR data and support for sm8550
>> iommu/arm-smmu: re-enable context caching in smmu reset operation
>>
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 143 ++++++++++++++++++++-
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 +
>
> Although it is not mentioned in the MAINTAINERS file, could you please
> CC linux-arm-msm when sending v2?
>
Sure Dmitry, will take care of this and CC linux-arm-msm while sending v2.
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 +
>> 4 files changed, 146 insertions(+), 9 deletions(-)
>>
>> --
>> 2.17.1
>>
>
>
On Mon, 6 Nov 2023 at 08:10, Bibek Kumar Patro
<[email protected]> wrote:
>
>
>
> On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
> > On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
> > <[email protected]> wrote:
> >>
> >>
> >>
> >> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> >>> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
> >>> <[email protected]> wrote:
> >>>>
> >>>> Add ACTLR data table for SM8550 along with support for
> >>>> same including SM8550 specific implementation operations.
> >>>>
> >>>> Signed-off-by: Bibek Kumar Patro <[email protected]>
> >>>> ---
> >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
> >>>> 1 file changed, 81 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> index 68c1f4908473..590b7c285299 100644
> >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> @@ -25,6 +25,64 @@ struct actlr_data {
> >>>> u32 actlr;
> >>>> };
> >>>>
> >>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> >>>> + { 0x18a0, 0x0000, 0x00000103 },
> >>>> + { 0x18e0, 0x0000, 0x00000103 },
> >>>> + { 0x0800, 0x0020, 0x00000001 },
> >>>> + { 0x1800, 0x00c0, 0x00000001 },
> >>>> + { 0x1820, 0x0000, 0x00000001 },
> >>>> + { 0x1860, 0x0000, 0x00000001 },
> >>>> + { 0x0c01, 0x0020, 0x00000303 },
> >>>> + { 0x0c02, 0x0020, 0x00000303 },
> >>>> + { 0x0c03, 0x0020, 0x00000303 },
> >>>> + { 0x0c04, 0x0020, 0x00000303 },
> >>>> + { 0x0c05, 0x0020, 0x00000303 },
> >>>> + { 0x0c06, 0x0020, 0x00000303 },
> >>>> + { 0x0c07, 0x0020, 0x00000303 },
> >>>> + { 0x0c08, 0x0020, 0x00000303 },
> >>>> + { 0x0c09, 0x0020, 0x00000303 },
> >>>> + { 0x0c0c, 0x0020, 0x00000303 },
> >>>> + { 0x0c0d, 0x0020, 0x00000303 },
> >>>> + { 0x0c0e, 0x0020, 0x00000303 },
> >>>> + { 0x0c0f, 0x0020, 0x00000303 },
> >>>> + { 0x1961, 0x0000, 0x00000303 },
> >>>> + { 0x1962, 0x0000, 0x00000303 },
> >>>> + { 0x1963, 0x0000, 0x00000303 },
> >>>> + { 0x1964, 0x0000, 0x00000303 },
> >>>> + { 0x1965, 0x0000, 0x00000303 },
> >>>> + { 0x1966, 0x0000, 0x00000303 },
> >>>> + { 0x1967, 0x0000, 0x00000303 },
> >>>> + { 0x1968, 0x0000, 0x00000303 },
> >>>> + { 0x1969, 0x0000, 0x00000303 },
> >>>> + { 0x196c, 0x0000, 0x00000303 },
> >>>> + { 0x196d, 0x0000, 0x00000303 },
> >>>> + { 0x196e, 0x0000, 0x00000303 },
> >>>> + { 0x196f, 0x0000, 0x00000303 },
> >>>> + { 0x19c1, 0x0010, 0x00000303 },
> >>>> + { 0x19c2, 0x0010, 0x00000303 },
> >>>> + { 0x19c3, 0x0010, 0x00000303 },
> >>>> + { 0x19c4, 0x0010, 0x00000303 },
> >>>> + { 0x19c5, 0x0010, 0x00000303 },
> >>>> + { 0x19c6, 0x0010, 0x00000303 },
> >>>> + { 0x19c7, 0x0010, 0x00000303 },
> >>>> + { 0x19c8, 0x0010, 0x00000303 },
> >>>> + { 0x19c9, 0x0010, 0x00000303 },
> >>>> + { 0x19cc, 0x0010, 0x00000303 },
> >>>> + { 0x19cd, 0x0010, 0x00000303 },
> >>>> + { 0x19ce, 0x0010, 0x00000303 },
> >>>> + { 0x19cf, 0x0010, 0x00000303 },
> >>>> + { 0x1c00, 0x0002, 0x00000103 },
> >>>> + { 0x1c01, 0x0000, 0x00000001 },
> >>>> + { 0x1920, 0x0000, 0x00000103 },
> >>>> + { 0x1923, 0x0000, 0x00000103 },
> >>>> + { 0x1924, 0x0000, 0x00000103 },
> >>>> + { 0x1940, 0x0000, 0x00000103 },
> >>>> + { 0x1941, 0x0004, 0x00000103 },
> >>>> + { 0x1943, 0x0000, 0x00000103 },
> >>>> + { 0x1944, 0x0000, 0x00000103 },
> >>>> + { 0x1947, 0x0000, 0x00000103 },
> >>>> +};
> >>>
> >>> This is nearly impossible to handle.
> >>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
> >>> adding comments for the devices.
> >>>
> >>
> >> Ack, Initially added the comments for devices, but since only driver is
> >> handling this data, and clients won't refer this so removed it. Will
> >> consider adding it back.
> >
> > It will help developers / porters who will try matching the SID and the device.
> >
>
> Agree on the same, I'll add those comments for devices back.
>
> >> This actlr field value might different (other
> >> than 0x1,0x103,0x303) in other platforms as per my anticipation,
> >> depending on the bit settings, so won't the defines change with
> >> different platforms? Hence this register setting value might be apt?
> >
> > It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
> > provide sensible defines so that we can understand and review them.
> >
>
> Understandable, In next patch I'll populate the actlr_data in
> following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
> where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
> the actlr values (0x1,0x103,0x303).
> This would help in understanding these values. Hope this
> proposed format will be okay?
Yes, this sounds good. Please also add description for CPRE and CMTLB bits.
>
>
> > Other platforms might use new defines.
> >
> >>
> >>>> +
> >>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> >>>> {
> >>>> return container_of(smmu, struct qcom_smmu, smmu);
> >>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
> >>>> .tlb_sync = qcom_smmu_tlb_sync,
> >>>> };
> >>>>
> >>>> +
> >>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> >>>> + .init_context = qcom_smmu_init_context,
> >>>> + .cfg_probe = qcom_smmu_cfg_probe,
> >>>> + .def_domain_type = qcom_smmu_def_domain_type,
> >>>> + .reset = arm_mmu500_reset,
> >>>> + .write_s2cr = qcom_smmu_write_s2cr,
> >>>> + .tlb_sync = qcom_smmu_tlb_sync,
> >>>> +};
> >>>> +
> >>>> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> >>>> .init_context = qcom_adreno_smmu_init_context,
> >>>> .def_domain_type = qcom_smmu_def_domain_type,
> >>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
> >>>> .reg_offset = qcom_smmu_impl0_reg_offset,
> >>>> };
> >>>>
> >>>> +static const struct actlr_config sm8550_actlrcfg = {
> >>>> + .adata = sm8550_apps_actlr_data,
> >>>> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> >>>> +};
> >>>> +
> >>>> /*
> >>>> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
> >>>> * there are not enough context banks.
> >>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
> >>>> /* Also no debug configuration. */
> >>>> };
> >>>>
> >>>> +
> >>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> >>>> + .impl = &sm8550_smmu_500_impl,
> >>>> + .adreno_impl = &qcom_adreno_smmu_500_impl,
> >>>> + .cfg = &qcom_smmu_impl0_cfg,
> >>>> + .actlrcfg = &sm8550_actlrcfg,
> >>>> +};
> >>>
> >>> This structure doesn't seem to be linked. Did you test your patches?
> >>>
> >>
> >> Apologies Dmitry, just checked back my patches, I tested it but while
> >> refining the patches I somehow missed this link
> >> { .compatible = "qcom,sm8550-smmu-500", .data =
> >> &sm8550_smmu_500_impl0_data };
> >> in below qcom_smmu_500_impl0_data structure.
> >> I will take care of this in next version.
> >>
> >>>> +
> >>>> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
> >>>> .impl = &qcom_smmu_500_impl,
> >>>> .adreno_impl = &qcom_adreno_smmu_500_impl,
> >>>> .cfg = &qcom_smmu_impl0_cfg,
> >>>> };
> >>>>
> >>>> -/*
> >>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> >>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
> >>>> - */
> >>>
> >>> Leave the comment in place.
> >>>
> >>
> >> Thanks for this comment which helped me to note the above mentioned
> >> mistake.
> >>
> >>>> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> >>>> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
> >>>> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> >>>> --
> >>>> 2.17.1
> >>>>
> >>>
> >>>
> >
> >
> >
--
With best wishes
Dmitry
On 11/6/2023 2:35 PM, Dmitry Baryshkov wrote:
> On Mon, 6 Nov 2023 at 08:10, Bibek Kumar Patro
> <[email protected]> wrote:
>>
>>
>>
>> On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
>>> On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
>>> <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> Add ACTLR data table for SM8550 along with support for
>>>>>> same including SM8550 specific implementation operations.
>>>>>>
>>>>>> Signed-off-by: Bibek Kumar Patro <[email protected]>
>>>>>> ---
>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>>>>>> 1 file changed, 81 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> index 68c1f4908473..590b7c285299 100644
>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> @@ -25,6 +25,64 @@ struct actlr_data {
>>>>>> u32 actlr;
>>>>>> };
>>>>>>
>>>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>>>>>> + { 0x18a0, 0x0000, 0x00000103 },
>>>>>> + { 0x18e0, 0x0000, 0x00000103 },
>>>>>> + { 0x0800, 0x0020, 0x00000001 },
>>>>>> + { 0x1800, 0x00c0, 0x00000001 },
>>>>>> + { 0x1820, 0x0000, 0x00000001 },
>>>>>> + { 0x1860, 0x0000, 0x00000001 },
>>>>>> + { 0x0c01, 0x0020, 0x00000303 },
>>>>>> + { 0x0c02, 0x0020, 0x00000303 },
>>>>>> + { 0x0c03, 0x0020, 0x00000303 },
>>>>>> + { 0x0c04, 0x0020, 0x00000303 },
>>>>>> + { 0x0c05, 0x0020, 0x00000303 },
>>>>>> + { 0x0c06, 0x0020, 0x00000303 },
>>>>>> + { 0x0c07, 0x0020, 0x00000303 },
>>>>>> + { 0x0c08, 0x0020, 0x00000303 },
>>>>>> + { 0x0c09, 0x0020, 0x00000303 },
>>>>>> + { 0x0c0c, 0x0020, 0x00000303 },
>>>>>> + { 0x0c0d, 0x0020, 0x00000303 },
>>>>>> + { 0x0c0e, 0x0020, 0x00000303 },
>>>>>> + { 0x0c0f, 0x0020, 0x00000303 },
>>>>>> + { 0x1961, 0x0000, 0x00000303 },
>>>>>> + { 0x1962, 0x0000, 0x00000303 },
>>>>>> + { 0x1963, 0x0000, 0x00000303 },
>>>>>> + { 0x1964, 0x0000, 0x00000303 },
>>>>>> + { 0x1965, 0x0000, 0x00000303 },
>>>>>> + { 0x1966, 0x0000, 0x00000303 },
>>>>>> + { 0x1967, 0x0000, 0x00000303 },
>>>>>> + { 0x1968, 0x0000, 0x00000303 },
>>>>>> + { 0x1969, 0x0000, 0x00000303 },
>>>>>> + { 0x196c, 0x0000, 0x00000303 },
>>>>>> + { 0x196d, 0x0000, 0x00000303 },
>>>>>> + { 0x196e, 0x0000, 0x00000303 },
>>>>>> + { 0x196f, 0x0000, 0x00000303 },
>>>>>> + { 0x19c1, 0x0010, 0x00000303 },
>>>>>> + { 0x19c2, 0x0010, 0x00000303 },
>>>>>> + { 0x19c3, 0x0010, 0x00000303 },
>>>>>> + { 0x19c4, 0x0010, 0x00000303 },
>>>>>> + { 0x19c5, 0x0010, 0x00000303 },
>>>>>> + { 0x19c6, 0x0010, 0x00000303 },
>>>>>> + { 0x19c7, 0x0010, 0x00000303 },
>>>>>> + { 0x19c8, 0x0010, 0x00000303 },
>>>>>> + { 0x19c9, 0x0010, 0x00000303 },
>>>>>> + { 0x19cc, 0x0010, 0x00000303 },
>>>>>> + { 0x19cd, 0x0010, 0x00000303 },
>>>>>> + { 0x19ce, 0x0010, 0x00000303 },
>>>>>> + { 0x19cf, 0x0010, 0x00000303 },
>>>>>> + { 0x1c00, 0x0002, 0x00000103 },
>>>>>> + { 0x1c01, 0x0000, 0x00000001 },
>>>>>> + { 0x1920, 0x0000, 0x00000103 },
>>>>>> + { 0x1923, 0x0000, 0x00000103 },
>>>>>> + { 0x1924, 0x0000, 0x00000103 },
>>>>>> + { 0x1940, 0x0000, 0x00000103 },
>>>>>> + { 0x1941, 0x0004, 0x00000103 },
>>>>>> + { 0x1943, 0x0000, 0x00000103 },
>>>>>> + { 0x1944, 0x0000, 0x00000103 },
>>>>>> + { 0x1947, 0x0000, 0x00000103 },
>>>>>> +};
>>>>>
>>>>> This is nearly impossible to handle.
>>>>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
>>>>> adding comments for the devices.
>>>>>
>>>>
>>>> Ack, Initially added the comments for devices, but since only driver is
>>>> handling this data, and clients won't refer this so removed it. Will
>>>> consider adding it back.
>>>
>>> It will help developers / porters who will try matching the SID and the device.
>>>
>>
>> Agree on the same, I'll add those comments for devices back.
>>
>>>> This actlr field value might different (other
>>>> than 0x1,0x103,0x303) in other platforms as per my anticipation,
>>>> depending on the bit settings, so won't the defines change with
>>>> different platforms? Hence this register setting value might be apt?
>>>
>>> It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
>>> provide sensible defines so that we can understand and review them.
>>>
>>
>> Understandable, In next patch I'll populate the actlr_data in
>> following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
>> where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
>> the actlr values (0x1,0x103,0x303).
>> This would help in understanding these values. Hope this
>> proposed format will be okay?
>
> Yes, this sounds good. Please also add description for CPRE and CMTLB bits.
>
Thanks for the confirmation. Sure will add the descriptions for
CPRE and CMTLB bits as well in v2
>>
>>
>>> Other platforms might use new defines.
>>>
>>>>
>>>>>> +
>>>>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>>>>> {
>>>>>> return container_of(smmu, struct qcom_smmu, smmu);
>>>>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>>>>>> .tlb_sync = qcom_smmu_tlb_sync,
>>>>>> };
>>>>>>
>>>>>> +
>>>>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>>>> + .init_context = qcom_smmu_init_context,
>>>>>> + .cfg_probe = qcom_smmu_cfg_probe,
>>>>>> + .def_domain_type = qcom_smmu_def_domain_type,
>>>>>> + .reset = arm_mmu500_reset,
>>>>>> + .write_s2cr = qcom_smmu_write_s2cr,
>>>>>> + .tlb_sync = qcom_smmu_tlb_sync,
>>>>>> +};
>>>>>> +
>>>>>> static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>>>> .init_context = qcom_adreno_smmu_init_context,
>>>>>> .def_domain_type = qcom_smmu_def_domain_type,
>>>>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>>>>>> .reg_offset = qcom_smmu_impl0_reg_offset,
>>>>>> };
>>>>>>
>>>>>> +static const struct actlr_config sm8550_actlrcfg = {
>>>>>> + .adata = sm8550_apps_actlr_data,
>>>>>> + .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>>>>>> +};
>>>>>> +
>>>>>> /*
>>>>>> * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>>>>>> * there are not enough context banks.
>>>>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>>>>>> /* Also no debug configuration. */
>>>>>> };
>>>>>>
>>>>>> +
>>>>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>>>>>> + .impl = &sm8550_smmu_500_impl,
>>>>>> + .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>> + .cfg = &qcom_smmu_impl0_cfg,
>>>>>> + .actlrcfg = &sm8550_actlrcfg,
>>>>>> +};
>>>>>
>>>>> This structure doesn't seem to be linked. Did you test your patches?
>>>>>
>>>>
>>>> Apologies Dmitry, just checked back my patches, I tested it but while
>>>> refining the patches I somehow missed this link
>>>> { .compatible = "qcom,sm8550-smmu-500", .data =
>>>> &sm8550_smmu_500_impl0_data };
>>>> in below qcom_smmu_500_impl0_data structure.
>>>> I will take care of this in next version.
>>>>
>>>>>> +
>>>>>> static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>>>>>> .impl = &qcom_smmu_500_impl,
>>>>>> .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>> .cfg = &qcom_smmu_impl0_cfg,
>>>>>> };
>>>>>>
>>>>>> -/*
>>>>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>>>>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>>>>>> - */
>>>>>
>>>>> Leave the comment in place.
>>>>>
>>>>
>>>> Thanks for this comment which helped me to note the above mentioned
>>>> mistake.
>>>>
>>>>>> static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>>>>>> { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>>>>>> { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
>
>
>