* [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs
@ 2023-11-14 13:56 Bibek Kumar Patro
2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro
` (3 more replies)
0 siblings, 4 replies; 34+ messages in thread
From: Bibek Kumar Patro @ 2023-11-14 13:56 UTC (permalink / raw)
To: will, robin.murphy, joro, dmitry.baryshkov, a39.skl,
konrad.dybcio, quic_pkondeti, quic_molvera
Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel,
qipl.kernel.upstream, Bibek Kumar Patro
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.
Changes in v2 from v1:
- Incorporated suggestions on v1 from Dmitry,Konrad,Pratyush.
- Added defines for ACTLR values.
- Linked sm8550 implementation structure to corresponding
compatible string.
- Repackaged actlr value set implementation to separate function.
- Fixed indentation errors.
- Link to v1: https://lore.kernel.org/all/20231103215124.1095-1-quic_bibekkum@quicinc.com/
Changes in v1 from RFC:
- Incorporated suggestion form Robin on RFC.
- Moved the actlr data table into driver, instead of maintaining
it inside soc specific DT and piggybacking on exisiting iommus
property (iommu = <SID, MASK, ACTLR>) to set this value during
smmu probe.
- Link to RFC: //lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/
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 | 156 ++++++++++++++++++++-
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, 159 insertions(+), 9 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 34+ messages in thread* [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 13:56 [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Bibek Kumar Patro @ 2023-11-14 13:56 ` Bibek Kumar Patro 2023-11-14 14:08 ` Dmitry Baryshkov 2023-11-14 16:55 ` Robin Murphy 2023-11-14 13:56 ` [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 Bibek Kumar Patro ` (2 subsequent siblings) 3 siblings, 2 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-14 13:56 UTC (permalink / raw) To: will, robin.murphy, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ 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, 51 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 549ae4dba3a6..578c662c7c30 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; + size_t 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); @@ -261,9 +272,36 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { { } }; +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, + const struct actlr_config *actlrcfg) +{ + struct arm_smmu_smr *smr = smmu->smrs; + int i; + u16 id; + u16 mask; + + 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); + } +} + 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; + int idx = smmu_domain->cfg.cbndx; + + if (qsmmu->actlrcfg) { + actlrcfg = qsmmu->actlrcfg; + arm_smmu_set_actlr(smmu, idx, actlrcfg); + } + smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; return 0; @@ -467,6 +505,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 d6d1a2a55cc0..8e4faf015286 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -990,9 +990,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro @ 2023-11-14 14:08 ` Dmitry Baryshkov 2023-11-15 5:42 ` Bibek Kumar Patro 2023-11-14 16:55 ` Robin Murphy 1 sibling, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-14 14:08 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro <quic_bibekkum@quicinc.com> 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 <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ > 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, 51 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 549ae4dba3a6..578c662c7c30 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; > + size_t size; Merge this into struct qcom_smmu_match_data. > +}; > + > +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); > @@ -261,9 +272,36 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { > { } > }; > > +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, > + const struct actlr_config *actlrcfg) > +{ > + struct arm_smmu_smr *smr = smmu->smrs; > + int i; > + u16 id; > + u16 mask; > + > + 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); > + } > +} > + > 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; > + int idx = smmu_domain->cfg.cbndx; > + > + if (qsmmu->actlrcfg) { > + actlrcfg = qsmmu->actlrcfg; > + arm_smmu_set_actlr(smmu, idx, actlrcfg); > + } > + > smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; > > return 0; > @@ -467,6 +505,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 d6d1a2a55cc0..8e4faf015286 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c > @@ -990,9 +990,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 14:08 ` Dmitry Baryshkov @ 2023-11-15 5:42 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 5:42 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/14/2023 7:38 PM, Dmitry Baryshkov wrote: > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> 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 <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ >> 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, 51 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 549ae4dba3a6..578c662c7c30 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; >> + size_t size; > > Merge this into struct qcom_smmu_match_data. > Just saw your response on the other thread in v1 patch, let me try again once to accomodate into single structure as suggested >> +}; >> + >> +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); >> @@ -261,9 +272,36 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { >> { } >> }; >> >> +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, >> + const struct actlr_config *actlrcfg) >> +{ >> + struct arm_smmu_smr *smr = smmu->smrs; >> + int i; >> + u16 id; >> + u16 mask; >> + >> + 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); >> + } >> +} >> + >> 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; >> + int idx = smmu_domain->cfg.cbndx; >> + >> + if (qsmmu->actlrcfg) { >> + actlrcfg = qsmmu->actlrcfg; >> + arm_smmu_set_actlr(smmu, idx, actlrcfg); >> + } >> + >> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; >> >> return 0; >> @@ -467,6 +505,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 d6d1a2a55cc0..8e4faf015286 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >> @@ -990,9 +990,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 regards, Bibek _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro 2023-11-14 14:08 ` Dmitry Baryshkov @ 2023-11-14 16:55 ` Robin Murphy 2023-11-15 12:32 ` Bibek Kumar Patro 2023-11-15 13:54 ` Bibek Kumar Patro 1 sibling, 2 replies; 34+ messages in thread From: Robin Murphy @ 2023-11-14 16:55 UTC (permalink / raw) To: Bibek Kumar Patro, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 14/11/2023 1:56 pm, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ > 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, 51 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 549ae4dba3a6..578c662c7c30 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; > + size_t size; > +}; > + > +struct actlr_data { > + u16 sid; > + u16 mask; Do we need to worry about masks? If you're already assuming that any SMR will be programmed to match a superset of the data here, surely a single unique ID per device would suffice? > + u32 actlr; > +}; > + > static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > { > return container_of(smmu, struct qcom_smmu, smmu); > @@ -261,9 +272,36 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { > { } > }; > > +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, > + const struct actlr_config *actlrcfg) > +{ > + struct arm_smmu_smr *smr = smmu->smrs; > + int i; > + u16 id; > + u16 mask; > + > + for (i = 0; i < actlrcfg->size; ++i) { > + id = actlrcfg->adata[i].sid; > + mask = actlrcfg->adata[i].mask; > + if (!smr_is_subset(*smr, id, mask)) How well have you tested this? ;) > + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR, > + actlrcfg->adata[i].actlr); > + } > +} > + > 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; > + int idx = smmu_domain->cfg.cbndx; > + > + if (qsmmu->actlrcfg) { > + actlrcfg = qsmmu->actlrcfg; > + arm_smmu_set_actlr(smmu, idx, actlrcfg); > + } > + > smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; > > return 0; > @@ -467,6 +505,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; Do we really need to replicate multiple parts of the data, or would it be sensible to just replace qsmmu->cfg with qsmmu->data and handle the further dereferences in the places that want them? > + > 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 d6d1a2a55cc0..8e4faf015286 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c > @@ -990,9 +990,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) Hmm, that name reads as implying the opposite of what it actually tests, not to mention that passing structs by value is a bit horrid as well :( Thanks, Robin. > +{ > + 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 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 16:55 ` Robin Murphy @ 2023-11-15 12:32 ` Bibek Kumar Patro 2023-11-15 15:04 ` Robin Murphy 2023-11-15 13:54 ` Bibek Kumar Patro 1 sibling, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 12:32 UTC (permalink / raw) To: Robin Murphy, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/14/2023 10:25 PM, Robin Murphy wrote: > On 14/11/2023 1:56 pm, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ >> 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, 51 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 549ae4dba3a6..578c662c7c30 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; >> + size_t size; >> +}; >> + >> +struct actlr_data { >> + u16 sid; >> + u16 mask; > > Do we need to worry about masks? If you're already assuming that any SMR > will be programmed to match a superset of the data here, surely a single > unique ID per device would suffice? > If you refer to the arm_smmu_set_actlr below, mask would be needed as we would check for mask along with sid as well while assigning actlr configuration. Also with mask no of actlr entries can be reduces as with out mask we have to have entry for each sid. >> + u32 actlr; >> +}; >> + >> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >> { >> return container_of(smmu, struct qcom_smmu, smmu); >> @@ -261,9 +272,36 @@ static const struct of_device_id >> qcom_smmu_client_of_match[] __maybe_unused = { >> { } >> }; >> >> +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, >> + const struct actlr_config *actlrcfg) >> +{ >> + struct arm_smmu_smr *smr = smmu->smrs; >> + int i; >> + u16 id; >> + u16 mask; >> + >> + for (i = 0; i < actlrcfg->size; ++i) { >> + id = actlrcfg->adata[i].sid; >> + mask = actlrcfg->adata[i].mask; >> + if (!smr_is_subset(*smr, id, mask)) > > How well have you tested this? ;) > Well, this logic has worked pretty good for us till now in our downstream implementation. :) (During testing as well this logic helped to better match the SMRs instead of manually mathcing the mask and SID which missed some SIDs) Also this is already being used to arm_smmu_find_sme hence packaged this logic in a wrapper to be used in other places as well(including ACTLR register setting case here.) >> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR, >> + actlrcfg->adata[i].actlr); >> + } >> +} >> + >> 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; >> + int idx = smmu_domain->cfg.cbndx; >> + >> + if (qsmmu->actlrcfg) { >> + actlrcfg = qsmmu->actlrcfg; >> + arm_smmu_set_actlr(smmu, idx, actlrcfg); >> + } >> + >> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; >> >> return 0; >> @@ -467,6 +505,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; > > Do we really need to replicate multiple parts of the data, or would it > be sensible to just replace qsmmu->cfg with qsmmu->data and handle the > further dereferences in the places that want them? > >> + >> 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 d6d1a2a55cc0..8e4faf015286 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >> @@ -990,9 +990,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) > > Hmm, that name reads as implying the opposite of what it actually tests, > not to mention that passing structs by value is a bit horrid as well :( > It might be okay to name it as subset_of_smr() though. You have any other naming suggestion in mind which could correctly describe the logic? Thanks & regards, Bibek > Thanks, > Robin. > >> +{ >> + 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 >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-15 12:32 ` Bibek Kumar Patro @ 2023-11-15 15:04 ` Robin Murphy 2023-11-20 10:24 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Robin Murphy @ 2023-11-15 15:04 UTC (permalink / raw) To: Bibek Kumar Patro, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 2023-11-15 12:32 pm, Bibek Kumar Patro wrote: > > > On 11/14/2023 10:25 PM, Robin Murphy wrote: >> On 14/11/2023 1:56 pm, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> >>> --- >>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 ++++++++++++++++++++++ >>> 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, 51 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 549ae4dba3a6..578c662c7c30 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; >>> + size_t size; >>> +}; >>> + >>> +struct actlr_data { >>> + u16 sid; >>> + u16 mask; >> >> Do we need to worry about masks? If you're already assuming that any >> SMR will be programmed to match a superset of the data here, surely a >> single unique ID per device would suffice? >> > > If you refer to the arm_smmu_set_actlr below, mask would be needed as we > would check for mask along with sid as well while assigning actlr > configuration. Also with mask no of actlr entries can be reduces as > with out mask we have to have entry for each sid. But why? As far as I can tell, the design in patch #2 is to have a single data entry for each distinct device, since you're expecting to see a single SMR programmed to match at least the full id/mask range of the entry. However any SMR which matches that full range will by definition also match any smaller subset of that range as well, so we can uniquely identify any device in this context from just any *one* of its IDs, thus we could store less data and have simpler matching logic. >>> + u32 actlr; >>> +}; >>> + >>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >>> { >>> return container_of(smmu, struct qcom_smmu, smmu); >>> @@ -261,9 +272,36 @@ static const struct of_device_id >>> qcom_smmu_client_of_match[] __maybe_unused = { >>> { } >>> }; >>> >>> +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, >>> + const struct actlr_config *actlrcfg) >>> +{ >>> + struct arm_smmu_smr *smr = smmu->smrs; >>> + int i; >>> + u16 id; >>> + u16 mask; >>> + >>> + for (i = 0; i < actlrcfg->size; ++i) { >>> + id = actlrcfg->adata[i].sid; >>> + mask = actlrcfg->adata[i].mask; >>> + if (!smr_is_subset(*smr, id, mask)) >> >> How well have you tested this? ;) >> > > Well, this logic has worked pretty good for us till now in our > downstream implementation. :) (During testing as well this logic helped > to better match the SMRs instead of manually mathcing the mask and SID > which missed some SIDs) > Also this is already being used to arm_smmu_find_sme hence packaged this > logic in a wrapper to be used in other places as well(including ACTLR > register setting case here.) 1) Look at what you're matching the id and mask values *against* 2) Look at the polarity of the condition 3) Consider the old saying "two wrongs don't make a right" >>> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR, >>> + actlrcfg->adata[i].actlr); >>> + } >>> +} >>> + >>> 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; >>> + int idx = smmu_domain->cfg.cbndx; >>> + >>> + if (qsmmu->actlrcfg) { >>> + actlrcfg = qsmmu->actlrcfg; >>> + arm_smmu_set_actlr(smmu, idx, actlrcfg); >>> + } >>> + >>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; >>> >>> return 0; >>> @@ -467,6 +505,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; >> >> Do we really need to replicate multiple parts of the data, or would it >> be sensible to just replace qsmmu->cfg with qsmmu->data and handle the >> further dereferences in the places that want them? >> >>> + >>> 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 d6d1a2a55cc0..8e4faf015286 100644 >>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>> @@ -990,9 +990,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) >> >> Hmm, that name reads as implying the opposite of what it actually >> tests, not to mention that passing structs by value is a bit horrid as >> well :( >> > > It might be okay to name it as subset_of_smr() though. You have any > other naming suggestion in mind which could correctly describe the > logic? As above I think the ideal answer is to avoid the reason for factoring it out at all, by using a simpler matching process to begin with. Thanks, Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-15 15:04 ` Robin Murphy @ 2023-11-20 10:24 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-20 10:24 UTC (permalink / raw) To: Robin Murphy, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 8:34 PM, Robin Murphy wrote: > On 2023-11-15 12:32 pm, Bibek Kumar Patro wrote: >> >> >> On 11/14/2023 10:25 PM, Robin Murphy wrote: >>> On 14/11/2023 1:56 pm, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 41 >>>> ++++++++++++++++++++++ >>>> 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, 51 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 549ae4dba3a6..578c662c7c30 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; >>>> + size_t size; >>>> +}; >>>> + >>>> +struct actlr_data { >>>> + u16 sid; >>>> + u16 mask; >>> >>> Do we need to worry about masks? If you're already assuming that any >>> SMR will be programmed to match a superset of the data here, surely a >>> single unique ID per device would suffice? >>> Thanks for this comment. I see now, only assuming smrs might not be able to correctly set the ACTLR values for all the context banks without the index. it should be smr = smmu->smrs[idx] instead of smr = smmu->smrs or it will be considering the single SMR for all the associated devices. >> >> If you refer to the arm_smmu_set_actlr below, mask would be needed as >> we would check for mask along with sid as well while assigning actlr >> configuration. Also with mask no of actlr entries can be reduces as >> with out mask we have to have entry for each sid. > > But why? As far as I can tell, the design in patch #2 is to have a > single data entry for each distinct device, since you're expecting to > see a single SMR programmed to match at least the full id/mask range of > the entry. However any SMR which matches that full range will by > definition also match any smaller subset of that range as well, so we > can uniquely identify any device in this context from just any *one* of > its IDs, thus we could store less data and have simpler matching logic. > I was going though your earlier reply once again and noticed we would need to mention the context bank index against the smr as well to take the right ACTLR value, as I replied to your earlier response above. >>>> + u32 actlr; >>>> +}; >>>> + >>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >>>> { >>>> return container_of(smmu, struct qcom_smmu, smmu); >>>> @@ -261,9 +272,36 @@ static const struct of_device_id >>>> qcom_smmu_client_of_match[] __maybe_unused = { >>>> { } >>>> }; >>>> >>>> +static void arm_smmu_set_actlr(struct arm_smmu_device *smmu, int idx, >>>> + const struct actlr_config *actlrcfg) >>>> +{ >>>> + struct arm_smmu_smr *smr = smmu->smrs; It would be smr = smmu->smrs[idx] instead of smr = smmu->smrs to get sid of individual device attached to corresponding context bank. >>>> + int i; >>>> + u16 id; >>>> + u16 mask; >>>> + >>>> + for (i = 0; i < actlrcfg->size; ++i) { >>>> + id = actlrcfg->adata[i].sid; >>>> + mask = actlrcfg->adata[i].mask; >>>> + if (!smr_is_subset(*smr, id, mask)) >>> >>> How well have you tested this? ;) >>> >> >> Well, this logic has worked pretty good for us till now in our >> downstream implementation. :) (During testing as well this logic >> helped to better match the SMRs instead of manually mathcing the mask >> and SID which missed some SIDs) >> Also this is already being used to arm_smmu_find_sme hence packaged this >> logic in a wrapper to be used in other places as well(including ACTLR >> register setting case here.) > > 1) Look at what you're matching the id and mask values *against* > Got this now. Will take care of this in next version. > 2) Look at the polarity of the condition > > 3) Consider the old saying "two wrongs don't make a right" > Noted. >>>> + arm_smmu_cb_write(smmu, idx, ARM_SMMU_CB_ACTLR, >>>> + actlrcfg->adata[i].actlr); >>>> + } >>>> +} >>>> + >>>> 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; >>>> + int idx = smmu_domain->cfg.cbndx; >>>> + >>>> + if (qsmmu->actlrcfg) { >>>> + actlrcfg = qsmmu->actlrcfg; >>>> + arm_smmu_set_actlr(smmu, idx, actlrcfg); >>>> + } >>>> + >>>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; >>>> >>>> return 0; >>>> @@ -467,6 +505,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; >>> >>> Do we really need to replicate multiple parts of the data, or would >>> it be sensible to just replace qsmmu->cfg with qsmmu->data and handle >>> the further dereferences in the places that want them? >>> >>>> + >>>> 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 d6d1a2a55cc0..8e4faf015286 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c >>>> @@ -990,9 +990,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) >>> >>> Hmm, that name reads as implying the opposite of what it actually >>> tests, not to mention that passing structs by value is a bit horrid >>> as well :( >>> >> >> It might be okay to name it as subset_of_smr() though. You have any >> other naming suggestion in mind which could correctly describe the >> logic? > > As above I think the ideal answer is to avoid the reason for factoring > it out at all, by using a simpler matching process to begin with. > As per the approach mentioned above to use the context bank index, this same approach should be okay to go forward with. Thanks Robin for you above comments, I have noted the inputs and will take care of this in next version. Thanks & regards, Bibek > Thanks, > Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-14 16:55 ` Robin Murphy 2023-11-15 12:32 ` Bibek Kumar Patro @ 2023-11-15 13:54 ` Bibek Kumar Patro 2023-11-15 14:53 ` Robin Murphy 1 sibling, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 13:54 UTC (permalink / raw) To: Robin Murphy, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream >> @@ -467,6 +505,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; > > Do we really need to replicate multiple parts of the data, or would it > be sensible to just replace qsmmu->cfg with qsmmu->data and handle the > further dereferences in the places that want them? > Mm, could not understand this properly. :( Could you help explain more please? As per my understanding aren't data and qsmmu different structures. qcom_smmu is a superset of arm_smmu housing additonal properties and qcom_smmu_match_data is kind of a superset of arm_smmu_impl with additional specific implmentations, so both needs to be in place? Apologies if I understood your statement incorrectly. >> + >> 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 { >> }; >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-15 13:54 ` Bibek Kumar Patro @ 2023-11-15 14:53 ` Robin Murphy 2023-11-17 5:27 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Robin Murphy @ 2023-11-15 14:53 UTC (permalink / raw) To: Bibek Kumar Patro, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 2023-11-15 1:54 pm, Bibek Kumar Patro wrote: > >>> @@ -467,6 +505,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; >> >> Do we really need to replicate multiple parts of the data, or would it >> be sensible to just replace qsmmu->cfg with qsmmu->data and handle the >> further dereferences in the places that want them? >> > > Mm, could not understand this properly. :( Could you help explain more > please? > As per my understanding aren't data and qsmmu different structures. > qcom_smmu is a superset of arm_smmu housing additonal properties > and qcom_smmu_match_data is kind of a superset of arm_smmu_impl with > additional specific implmentations, so both needs to be in place? > Apologies if I understood your statement incorrectly. My point is that the data is static and constant, so there's really no point storing multiple pointers into different bits of it. So rather than: qsmmu->cfg = data->cfg; qssmu->actlrcfg = data->actlrcfg; ... do_something(qsmmu->cfg); ... do_other_thing(qsmmu->actlrcfg); we can just store the one pointer and have: qsmmu->data = data; ... do_something(qsmmu->data->cfg); ... do_other_thing(qsmmu->data->actlrcfg); Thanks, Robin. >>> + >>> 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 { >>> }; >>> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings 2023-11-15 14:53 ` Robin Murphy @ 2023-11-17 5:27 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-17 5:27 UTC (permalink / raw) To: Robin Murphy, will, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 8:23 PM, Robin Murphy wrote: > On 2023-11-15 1:54 pm, Bibek Kumar Patro wrote: >> >>>> @@ -467,6 +505,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; >>> >>> Do we really need to replicate multiple parts of the data, or would >>> it be sensible to just replace qsmmu->cfg with qsmmu->data and handle >>> the further dereferences in the places that want them? >>> >> >> Mm, could not understand this properly. :( Could you help explain more >> please? >> As per my understanding aren't data and qsmmu different structures. >> qcom_smmu is a superset of arm_smmu housing additonal properties >> and qcom_smmu_match_data is kind of a superset of arm_smmu_impl with >> additional specific implmentations, so both needs to be in place? >> Apologies if I understood your statement incorrectly. > > My point is that the data is static and constant, so there's really no > point storing multiple pointers into different bits of it. So rather than: > > qsmmu->cfg = data->cfg; > qssmu->actlrcfg = data->actlrcfg; > ... > do_something(qsmmu->cfg); > ... > do_other_thing(qsmmu->actlrcfg); > > we can just store the one pointer and have: > > qsmmu->data = data; > ... > do_something(qsmmu->data->cfg); > ... > do_other_thing(qsmmu->data->actlrcfg); > > Thanks, > Robin. > I see, this looks like probably we need a separate patch altogether for this cleanup, as "cfg" is used in other fault handling places as well as i can see and is introduced as a part of different patch. Should we scope this work for a separate patch if it's okay? Thanks, Bibek >>>> + >>>> 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 { >>>> }; >>>> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-14 13:56 [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro @ 2023-11-14 13:56 ` Bibek Kumar Patro 2023-11-14 14:12 ` Dmitry Baryshkov 2023-11-14 13:56 ` [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation Bibek Kumar Patro 2023-11-14 14:06 ` [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Dmitry Baryshkov 3 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-14 13:56 UTC (permalink / raw) To: will, robin.murphy, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream, Bibek Kumar Patro Add ACTLR data table for SM8550 along with support for same including SM8550 specific implementation operations. Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -25,6 +25,70 @@ struct actlr_data { u32 actlr; }; +#define PRE_FETCH_1 0 +#define PRE_FETCH_2 BIT(8) +#define PRE_FETCH_3 (BIT(9) | BIT(8)) +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ + +static const struct actlr_data sm8550_apps_actlr_data[] = { + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, +}; + static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) { return container_of(smmu, struct qcom_smmu, smmu); @@ -459,6 +523,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, @@ -522,6 +596,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. @@ -545,16 +624,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 }, @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, { } }; -- 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-14 13:56 ` [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 Bibek Kumar Patro @ 2023-11-14 14:12 ` Dmitry Baryshkov 2023-11-15 9:22 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-14 14:12 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > Add ACTLR data table for SM8550 along with support for > same including SM8550 specific implementation operations. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- > 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -25,6 +25,70 @@ struct actlr_data { > u32 actlr; > }; > > +#define PRE_FETCH_1 0 > +#define PRE_FETCH_2 BIT(8) > +#define PRE_FETCH_3 (BIT(9) | BIT(8)) What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And PRE_FETCH_1? Are these real numbers that refer to some amount / count or just dummy names? > +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ > +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ > + > +static const struct actlr_data sm8550_apps_actlr_data[] = { > + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, > + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, > + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, > + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, > + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, > + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > +}; > + > static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > { > return container_of(smmu, struct qcom_smmu, smmu); > @@ -459,6 +523,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, What is the difference between this one and qcom_smmu_500_impl ? > +}; > + > 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, > @@ -522,6 +596,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. > @@ -545,16 +624,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. > - */ NAK, leave this 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 }, > @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { > { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, > { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, > { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, > + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, > { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, > { } > }; > -- > 2.17.1 > -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-14 14:12 ` Dmitry Baryshkov @ 2023-11-15 9:22 ` Bibek Kumar Patro 2023-11-15 9:38 ` Dmitry Baryshkov 0 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 9:22 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> Add ACTLR data table for SM8550 along with support for >> same including SM8550 specific implementation operations. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- >> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -25,6 +25,70 @@ struct actlr_data { >> u32 actlr; >> }; >> >> +#define PRE_FETCH_1 0 >> +#define PRE_FETCH_2 BIT(8) >> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) > > What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And > PRE_FETCH_1? Are these real numbers that refer to some amount / count > or just dummy names? > No,these are not real numbers, but prefetch settings for a particular perfect configuration. >> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ >> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ >> + >> +static const struct actlr_data sm8550_apps_actlr_data[] = { >> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, >> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, >> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, >> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, >> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, >> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >> +}; >> + >> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >> { >> return container_of(smmu, struct qcom_smmu, smmu); >> @@ -459,6 +523,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, > > What is the difference between this one and qcom_smmu_500_impl ? > Noted, will remove this and use qcom_smmu_500_impl instead. Thanks for pointing this out. Since inititally the reset ops was different to reset CPRE bit only for sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it should be fine to use qcom_smmu_500_impl as there's no difference now. >> +}; >> + >> 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, >> @@ -522,6 +596,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. >> @@ -545,16 +624,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. >> - */ > > NAK, leave this in place. > Ack, will address this in next version. >> 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 }, >> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { >> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, >> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, >> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, >> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, >> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, >> { } >> }; >> -- >> 2.17.1 >> > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 9:22 ` Bibek Kumar Patro @ 2023-11-15 9:38 ` Dmitry Baryshkov 2023-11-15 9:51 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-15 9:38 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: > > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> Add ACTLR data table for SM8550 along with support for > >> same including SM8550 specific implementation operations. > >> > >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >> --- > >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- > >> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 > >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> @@ -25,6 +25,70 @@ struct actlr_data { > >> u32 actlr; > >> }; > >> > >> +#define PRE_FETCH_1 0 > >> +#define PRE_FETCH_2 BIT(8) > >> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) > > > > What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And > > PRE_FETCH_1? Are these real numbers that refer to some amount / count > > or just dummy names? > > > > No,these are not real numbers, but prefetch settings for a particular > perfect configuration. Then I'd ask for some better names or descriptions. > > >> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ > >> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ > >> + > >> +static const struct actlr_data sm8550_apps_actlr_data[] = { > >> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, > >> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, > >> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, > >> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, > >> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, > >> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >> +}; > >> + > >> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > >> { > >> return container_of(smmu, struct qcom_smmu, smmu); > >> @@ -459,6 +523,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, > > > > What is the difference between this one and qcom_smmu_500_impl ? > > > > Noted, will remove this and use qcom_smmu_500_impl instead. > Thanks for pointing this out. > Since inititally the reset ops was different to reset CPRE bit only for > sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset > ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it > should be fine to use qcom_smmu_500_impl as there's no difference now. > > >> +}; > >> + > >> 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, > >> @@ -522,6 +596,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. > >> @@ -545,16 +624,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. > >> - */ > > > > NAK, leave this in place. > > > > Ack, will address this in next version. > > >> 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 }, > >> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { > >> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, > >> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, > >> { } > >> }; > >> -- > >> 2.17.1 > >> > > > > -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 9:38 ` Dmitry Baryshkov @ 2023-11-15 9:51 ` Bibek Kumar Patro 2023-11-15 10:45 ` Dmitry Baryshkov 0 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 9:51 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> Add ACTLR data table for SM8550 along with support for >>>> same including SM8550 specific implementation operations. >>>> >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- >>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> @@ -25,6 +25,70 @@ struct actlr_data { >>>> u32 actlr; >>>> }; >>>> >>>> +#define PRE_FETCH_1 0 >>>> +#define PRE_FETCH_2 BIT(8) >>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) >>> >>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And >>> PRE_FETCH_1? Are these real numbers that refer to some amount / count >>> or just dummy names? >>> >> >> No,these are not real numbers, but prefetch settings for a particular >> perfect configuration. > > Then I'd ask for some better names or descriptions. > Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name in the following case. Would it be okay to use this name instead? >> >>>> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ >>>> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ >>>> + >>>> +static const struct actlr_data sm8550_apps_actlr_data[] = { >>>> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, >>>> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, >>>> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, >>>> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, >>>> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, >>>> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>> +}; >>>> + >>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >>>> { >>>> return container_of(smmu, struct qcom_smmu, smmu); >>>> @@ -459,6 +523,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, >>> >>> What is the difference between this one and qcom_smmu_500_impl ? >>> >> >> Noted, will remove this and use qcom_smmu_500_impl instead. >> Thanks for pointing this out. >> Since inititally the reset ops was different to reset CPRE bit only for >> sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset >> ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it >> should be fine to use qcom_smmu_500_impl as there's no difference now. >> >>>> +}; >>>> + >>>> 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, >>>> @@ -522,6 +596,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. >>>> @@ -545,16 +624,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. >>>> - */ >>> >>> NAK, leave this in place. >>> >> >> Ack, will address this in next version. >> >>>> 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 }, >>>> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { >>>> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, >>>> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>> { } >>>> }; >>>> -- >>>> 2.17.1 >>>> >>> >>> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 9:51 ` Bibek Kumar Patro @ 2023-11-15 10:45 ` Dmitry Baryshkov 2023-11-15 12:49 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-15 10:45 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: > > On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> > >> > >> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: > >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > >>> <quic_bibekkum@quicinc.com> wrote: > >>>> > >>>> Add ACTLR data table for SM8550 along with support for > >>>> same including SM8550 specific implementation operations. > >>>> > >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >>>> --- > >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- > >>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 > >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> @@ -25,6 +25,70 @@ struct actlr_data { > >>>> u32 actlr; > >>>> }; > >>>> > >>>> +#define PRE_FETCH_1 0 > >>>> +#define PRE_FETCH_2 BIT(8) > >>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) > >>> > >>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And > >>> PRE_FETCH_1? Are these real numbers that refer to some amount / count > >>> or just dummy names? > >>> > >> > >> No,these are not real numbers, but prefetch settings for a particular > >> perfect configuration. > > > > Then I'd ask for some better names or descriptions. > > > > Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name > in the following case. Would it be okay to use this name instead? Not really. > > >> > >>>> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ > >>>> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ > >>>> + > >>>> +static const struct actlr_data sm8550_apps_actlr_data[] = { > >>>> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, > >>>> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, > >>>> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>> +}; > >>>> + > >>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > >>>> { > >>>> return container_of(smmu, struct qcom_smmu, smmu); > >>>> @@ -459,6 +523,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, > >>> > >>> What is the difference between this one and qcom_smmu_500_impl ? > >>> > >> > >> Noted, will remove this and use qcom_smmu_500_impl instead. > >> Thanks for pointing this out. > >> Since inititally the reset ops was different to reset CPRE bit only for > >> sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset > >> ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it > >> should be fine to use qcom_smmu_500_impl as there's no difference now. > >> > >>>> +}; > >>>> + > >>>> 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, > >>>> @@ -522,6 +596,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. > >>>> @@ -545,16 +624,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. > >>>> - */ > >>> > >>> NAK, leave this in place. > >>> > >> > >> Ack, will address this in next version. > >> > >>>> 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 }, > >>>> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { > >>>> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, > >>>> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>> { } > >>>> }; > >>>> -- > >>>> 2.17.1 > >>>> > >>> > >>> > > > > > > -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 10:45 ` Dmitry Baryshkov @ 2023-11-15 12:49 ` Bibek Kumar Patro 2023-11-15 13:10 ` Dmitry Baryshkov 2023-11-15 16:42 ` Konrad Dybcio 0 siblings, 2 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 12:49 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: >>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> >>>> >>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: >>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>> >>>>>> Add ACTLR data table for SM8550 along with support for >>>>>> same including SM8550 specific implementation operations. >>>>>> >>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>> --- >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- >>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> @@ -25,6 +25,70 @@ struct actlr_data { >>>>>> u32 actlr; >>>>>> }; >>>>>> >>>>>> +#define PRE_FETCH_1 0 >>>>>> +#define PRE_FETCH_2 BIT(8) >>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) >>>>> >>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And >>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / count >>>>> or just dummy names? >>>>> >>>> >>>> No,these are not real numbers, but prefetch settings for a particular >>>> perfect configuration. >>> >>> Then I'd ask for some better names or descriptions. >>> >> >> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name >> in the following case. Would it be okay to use this name instead? > > Not really. > Any suggestion you have in mind, if not this nomenclature? >> >>>> >>>>>> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ >>>>>> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ >>>>>> + >>>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = { >>>>>> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, >>>>>> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, >>>>>> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, >>>>>> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, >>>>>> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, >>>>>> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, >>>>>> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, >>>>>> +}; >>>>>> + >>>>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) >>>>>> { >>>>>> return container_of(smmu, struct qcom_smmu, smmu); >>>>>> @@ -459,6 +523,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, >>>>> >>>>> What is the difference between this one and qcom_smmu_500_impl ? >>>>> >>>> >>>> Noted, will remove this and use qcom_smmu_500_impl instead. >>>> Thanks for pointing this out. >>>> Since inititally the reset ops was different to reset CPRE bit only for >>>> sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset >>>> ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it >>>> should be fine to use qcom_smmu_500_impl as there's no difference now. >>>> >>>>>> +}; >>>>>> + >>>>>> 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, >>>>>> @@ -522,6 +596,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. >>>>>> @@ -545,16 +624,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. >>>>>> - */ >>>>> >>>>> NAK, leave this in place. >>>>> >>>> >>>> Ack, will address this in next version. >>>> >>>>>> 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 }, >>>>>> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { >>>>>> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>>>> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>>>> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>>>> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, >>>>>> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, >>>>>> { } >>>>>> }; >>>>>> -- >>>>>> 2.17.1 >>>>>> >>>>> >>>>> >>> >>> >>> > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 12:49 ` Bibek Kumar Patro @ 2023-11-15 13:10 ` Dmitry Baryshkov 2023-11-15 16:42 ` Konrad Dybcio 1 sibling, 0 replies; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-15 13:10 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Wed, 15 Nov 2023 at 14:49, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: > > On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> > >> > >> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: > >>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro > >>> <quic_bibekkum@quicinc.com> wrote: > >>>> > >>>> > >>>> > >>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: > >>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > >>>>> <quic_bibekkum@quicinc.com> wrote: > >>>>>> > >>>>>> Add ACTLR data table for SM8550 along with support for > >>>>>> same including SM8550 specific implementation operations. > >>>>>> > >>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >>>>>> --- > >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- > >>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 > >>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>>>> @@ -25,6 +25,70 @@ struct actlr_data { > >>>>>> u32 actlr; > >>>>>> }; > >>>>>> > >>>>>> +#define PRE_FETCH_1 0 > >>>>>> +#define PRE_FETCH_2 BIT(8) > >>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) > >>>>> > >>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And > >>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / count > >>>>> or just dummy names? > >>>>> > >>>> > >>>> No,these are not real numbers, but prefetch settings for a particular > >>>> perfect configuration. > >>> > >>> Then I'd ask for some better names or descriptions. > >>> > >> > >> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name > >> in the following case. Would it be okay to use this name instead? > > > > Not really. > > > > Any suggestion you have in mind, if not this nomenclature? PREFETCH_something, PREFETCH_do_other_thing and PREFETCH_something_else > > >> > >>>> > >>>>>> +#define CPRE BIT(1) /* Enable context caching in the prefetch buffer */ > >>>>>> +#define CMTLB BIT(0) /* Enable context caching in the macro TLB */ > >>>>>> + > >>>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = { > >>>>>> + { 0x18a0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x18e0, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x0800, 0x0020, PRE_FETCH_1 | CMTLB }, > >>>>>> + { 0x1800, 0x00c0, PRE_FETCH_1 | CMTLB }, > >>>>>> + { 0x1820, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>>>> + { 0x1860, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>>>> + { 0x0c01, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c02, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c03, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c04, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c05, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c06, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c07, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c08, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c09, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c0c, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c0d, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c0e, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x0c0f, 0x0020, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1961, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1962, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1963, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1964, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1965, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1966, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1967, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1968, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1969, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x196c, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x196d, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x196e, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x196f, 0x0000, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c1, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c2, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c3, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c4, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c5, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c6, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c7, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c8, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19c9, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19cc, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19cd, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19ce, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x19cf, 0x0010, PRE_FETCH_3 | CPRE | CMTLB }, > >>>>>> + { 0x1c00, 0x0002, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1c01, 0x0000, PRE_FETCH_1 | CMTLB }, > >>>>>> + { 0x1920, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1923, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1924, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1940, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1941, 0x0004, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1943, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1944, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> + { 0x1947, 0x0000, PRE_FETCH_2 | CPRE | CMTLB }, > >>>>>> +}; > >>>>>> + > >>>>>> static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu) > >>>>>> { > >>>>>> return container_of(smmu, struct qcom_smmu, smmu); > >>>>>> @@ -459,6 +523,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, > >>>>> > >>>>> What is the difference between this one and qcom_smmu_500_impl ? > >>>>> > >>>> > >>>> Noted, will remove this and use qcom_smmu_500_impl instead. > >>>> Thanks for pointing this out. > >>>> Since inititally the reset ops was different to reset CPRE bit only for > >>>> sm8550 SoC hence sm8550_smmu_500_impl is defined, but now default reset > >>>> ops is modified to set CPRE bit for all SoCs ([PATCH v2 3/3]) so it > >>>> should be fine to use qcom_smmu_500_impl as there's no difference now. > >>>> > >>>>>> +}; > >>>>>> + > >>>>>> 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, > >>>>>> @@ -522,6 +596,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. > >>>>>> @@ -545,16 +624,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. > >>>>>> - */ > >>>>> > >>>>> NAK, leave this in place. > >>>>> > >>>> > >>>> Ack, will address this in next version. > >>>> > >>>>>> 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 }, > >>>>>> @@ -579,6 +662,7 @@ static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { > >>>>>> { .compatible = "qcom,sm8250-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>>>> { .compatible = "qcom,sm8350-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>>>> { .compatible = "qcom,sm8450-smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>>>> + { .compatible = "qcom,sm8550-smmu-500", .data = &sm8550_smmu_500_impl0_data }, > >>>>>> { .compatible = "qcom,smmu-500", .data = &qcom_smmu_500_impl0_data }, > >>>>>> { } > >>>>>> }; > >>>>>> -- > >>>>>> 2.17.1 > >>>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > > -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 12:49 ` Bibek Kumar Patro 2023-11-15 13:10 ` Dmitry Baryshkov @ 2023-11-15 16:42 ` Konrad Dybcio 2023-11-16 6:09 ` Bibek Kumar Patro 1 sibling, 1 reply; 34+ messages in thread From: Konrad Dybcio @ 2023-11-15 16:42 UTC (permalink / raw) To: Bibek Kumar Patro, Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/23 13:49, Bibek Kumar Patro wrote: > > > On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: >> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro >> <quic_bibekkum@quicinc.com> wrote: >>> >>> >>> >>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: >>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro >>>> <quic_bibekkum@quicinc.com> wrote: >>>>> >>>>> >>>>> >>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: >>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>>> >>>>>>> Add ACTLR data table for SM8550 along with support for >>>>>>> same including SM8550 specific implementation operations. >>>>>>> >>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>>> --- >>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 +++++++++++++++++++++- >>>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> @@ -25,6 +25,70 @@ struct actlr_data { >>>>>>> u32 actlr; >>>>>>> }; >>>>>>> >>>>>>> +#define PRE_FETCH_1 0 >>>>>>> +#define PRE_FETCH_2 BIT(8) >>>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) >>>>>> >>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And >>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / count >>>>>> or just dummy names? >>>>>> >>>>> >>>>> No,these are not real numbers, but prefetch settings for a particular >>>>> perfect configuration. >>>> >>>> Then I'd ask for some better names or descriptions. >>>> >>> >>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name >>> in the following case. Would it be okay to use this name instead? >> >> Not really. >> > > Any suggestion you have in mind, if not this nomenclature? Dmitry's concern seems to be that you provide: PRE_FETCH_1 /* prefetcher with settings preset no. 1 */ PRE_FETCH_2 /* prefetcher with settings preset no. 2 */ PRE_FETCH_3 /* prefetcher with settings preset no. 3 */ whereas it would be both useful and interesting to see what these settings mean, i.e. what differences there are between all of these presets. Konrad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-15 16:42 ` Konrad Dybcio @ 2023-11-16 6:09 ` Bibek Kumar Patro 2023-11-16 6:51 ` Dmitry Baryshkov 0 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-16 6:09 UTC (permalink / raw) To: Konrad Dybcio, Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 10:12 PM, Konrad Dybcio wrote: > > > On 11/15/23 13:49, Bibek Kumar Patro wrote: >> >> >> On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: >>> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> >>>> >>>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: >>>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro >>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: >>>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>>>> >>>>>>>> Add ACTLR data table for SM8550 along with support for >>>>>>>> same including SM8550 specific implementation operations. >>>>>>>> >>>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>>>> --- >>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 >>>>>>>> +++++++++++++++++++++- >>>>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>>> @@ -25,6 +25,70 @@ struct actlr_data { >>>>>>>> u32 actlr; >>>>>>>> }; >>>>>>>> >>>>>>>> +#define PRE_FETCH_1 0 >>>>>>>> +#define PRE_FETCH_2 BIT(8) >>>>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) >>>>>>> >>>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And >>>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / >>>>>>> count >>>>>>> or just dummy names? >>>>>>> >>>>>> >>>>>> No,these are not real numbers, but prefetch settings for a particular >>>>>> perfect configuration. >>>>> >>>>> Then I'd ask for some better names or descriptions. >>>>> >>>> >>>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name >>>> in the following case. Would it be okay to use this name instead? >>> >>> Not really. >>> >> >> Any suggestion you have in mind, if not this nomenclature? > Dmitry's concern seems to be that you provide: > > PRE_FETCH_1 /* prefetcher with settings preset no. 1 */ > PRE_FETCH_2 /* prefetcher with settings preset no. 2 */ > PRE_FETCH_3 /* prefetcher with settings preset no. 3 */ > > whereas it would be both useful and interesting to see what these > settings mean, i.e. what differences there are between all of > these presets. > Ah, okay got it now from Dimitry and yours' response. But we exactly won't be able to reveal what each of these settings mean, as this might risk of revealing IP as ACTLR bits are implementation defined (except CPRE and CMTLB) which other SoC vendors might be using it in different manner(or different purpose) in their downstream implementation. We can name it like (e.g PREFETCH_DISABLE, PREFETCH_SHALLOW, PREFETCH_DEEP) to indicate the behaviour, but won't be exactly name/describe it to explain what it does with a particular setting. > Konrad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-16 6:09 ` Bibek Kumar Patro @ 2023-11-16 6:51 ` Dmitry Baryshkov 2023-11-17 5:36 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-16 6:51 UTC (permalink / raw) To: Bibek Kumar Patro Cc: Konrad Dybcio, will, robin.murphy, joro, a39.skl, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Thu, 16 Nov 2023 at 08:10, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/15/2023 10:12 PM, Konrad Dybcio wrote: > > > > > > On 11/15/23 13:49, Bibek Kumar Patro wrote: > >> > >> > >> On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: > >>> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro > >>> <quic_bibekkum@quicinc.com> wrote: > >>>> > >>>> > >>>> > >>>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: > >>>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro > >>>>> <quic_bibekkum@quicinc.com> wrote: > >>>>>> > >>>>>> > >>>>>> > >>>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: > >>>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > >>>>>>> <quic_bibekkum@quicinc.com> wrote: > >>>>>>>> > >>>>>>>> Add ACTLR data table for SM8550 along with support for > >>>>>>>> same including SM8550 specific implementation operations. > >>>>>>>> > >>>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >>>>>>>> --- > >>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 > >>>>>>>> +++++++++++++++++++++- > >>>>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 > >>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>>>>>> @@ -25,6 +25,70 @@ struct actlr_data { > >>>>>>>> u32 actlr; > >>>>>>>> }; > >>>>>>>> > >>>>>>>> +#define PRE_FETCH_1 0 > >>>>>>>> +#define PRE_FETCH_2 BIT(8) > >>>>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) > >>>>>>> > >>>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And > >>>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / > >>>>>>> count > >>>>>>> or just dummy names? > >>>>>>> > >>>>>> > >>>>>> No,these are not real numbers, but prefetch settings for a particular > >>>>>> perfect configuration. > >>>>> > >>>>> Then I'd ask for some better names or descriptions. > >>>>> > >>>> > >>>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name > >>>> in the following case. Would it be okay to use this name instead? > >>> > >>> Not really. > >>> > >> > >> Any suggestion you have in mind, if not this nomenclature? > > Dmitry's concern seems to be that you provide: > > > > PRE_FETCH_1 /* prefetcher with settings preset no. 1 */ > > PRE_FETCH_2 /* prefetcher with settings preset no. 2 */ > > PRE_FETCH_3 /* prefetcher with settings preset no. 3 */ > > > > whereas it would be both useful and interesting to see what these > > settings mean, i.e. what differences there are between all of > > these presets. > > > > Ah, okay got it now from Dimitry and yours' response. > But we exactly won't be able to reveal what each of these settings > mean, as this might risk of revealing IP as ACTLR bits are > implementation defined (except CPRE and CMTLB) which other SoC vendors > might be using it in different manner(or different purpose) in their > downstream implementation. > We can name it like (e.g PREFETCH_DISABLE, PREFETCH_SHALLOW, > PREFETCH_DEEP) to indicate the behaviour, but won't be exactly > name/describe it to explain what it does with a particular setting. This is already better than 1,2,3. -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 2023-11-16 6:51 ` Dmitry Baryshkov @ 2023-11-17 5:36 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-17 5:36 UTC (permalink / raw) To: Dmitry Baryshkov Cc: Konrad Dybcio, will, robin.murphy, joro, a39.skl, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/16/2023 12:21 PM, Dmitry Baryshkov wrote: > On Thu, 16 Nov 2023 at 08:10, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/15/2023 10:12 PM, Konrad Dybcio wrote: >>> >>> >>> On 11/15/23 13:49, Bibek Kumar Patro wrote: >>>> >>>> >>>> On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote: >>>>> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro >>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote: >>>>>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro >>>>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote: >>>>>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>>>>>> <quic_bibekkum@quicinc.com> wrote: >>>>>>>>>> >>>>>>>>>> Add ACTLR data table for SM8550 along with support for >>>>>>>>>> same including SM8550 specific implementation operations. >>>>>>>>>> >>>>>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>>>>>> --- >>>>>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 92 >>>>>>>>>> +++++++++++++++++++++- >>>>>>>>>> 1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644 >>>>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>>>>> @@ -25,6 +25,70 @@ struct actlr_data { >>>>>>>>>> u32 actlr; >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> +#define PRE_FETCH_1 0 >>>>>>>>>> +#define PRE_FETCH_2 BIT(8) >>>>>>>>>> +#define PRE_FETCH_3 (BIT(9) | BIT(8)) >>>>>>>>> >>>>>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And >>>>>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / >>>>>>>>> count >>>>>>>>> or just dummy names? >>>>>>>>> >>>>>>>> >>>>>>>> No,these are not real numbers, but prefetch settings for a particular >>>>>>>> perfect configuration. >>>>>>> >>>>>>> Then I'd ask for some better names or descriptions. >>>>>>> >>>>>> >>>>>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name >>>>>> in the following case. Would it be okay to use this name instead? >>>>> >>>>> Not really. >>>>> >>>> >>>> Any suggestion you have in mind, if not this nomenclature? >>> Dmitry's concern seems to be that you provide: >>> >>> PRE_FETCH_1 /* prefetcher with settings preset no. 1 */ >>> PRE_FETCH_2 /* prefetcher with settings preset no. 2 */ >>> PRE_FETCH_3 /* prefetcher with settings preset no. 3 */ >>> >>> whereas it would be both useful and interesting to see what these >>> settings mean, i.e. what differences there are between all of >>> these presets. >>> >> >> Ah, okay got it now from Dimitry and yours' response. >> But we exactly won't be able to reveal what each of these settings >> mean, as this might risk of revealing IP as ACTLR bits are >> implementation defined (except CPRE and CMTLB) which other SoC vendors >> might be using it in different manner(or different purpose) in their >> downstream implementation. >> We can name it like (e.g PREFETCH_DISABLE, PREFETCH_SHALLOW, >> PREFETCH_DEEP) to indicate the behaviour, but won't be exactly >> name/describe it to explain what it does with a particular setting. > > This is already better than 1,2,3. > Acked, will take care of this in next version. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-14 13:56 [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 Bibek Kumar Patro @ 2023-11-14 13:56 ` Bibek Kumar Patro 2023-11-14 14:15 ` Dmitry Baryshkov 2023-11-15 16:43 ` Konrad Dybcio 2023-11-14 14:06 ` [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Dmitry Baryshkov 3 siblings, 2 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-14 13:56 UTC (permalink / raw) To: will, robin.murphy, joro, dmitry.baryshkov, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream, Bibek Kumar Patro 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 <quic_bibekkum@quicinc.com> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) return match ? IOMMU_DOMAIN_IDENTITY : 0; } +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) +{ + int i; + u32 reg; + + arm_mmu500_reset(smmu); + + /* Re-enable context caching after reset */ + for (i = 0; i < smmu->num_context_banks; ++i) { + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); + reg |= 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, @@ -509,7 +526,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, }; @@ -528,7 +545,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, }; @@ -544,7 +561,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-14 13:56 ` [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation Bibek Kumar Patro @ 2023-11-14 14:15 ` Dmitry Baryshkov 2023-11-15 9:45 ` Bibek Kumar Patro 2023-11-15 16:43 ` Konrad Dybcio 1 sibling, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-14 14:15 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro <quic_bibekkum@quicinc.com> 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. Please refer to Documentation/process/submitting-patches.rst and rephrase this following the rules there. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > return match ? IOMMU_DOMAIN_IDENTITY : 0; > } > > +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > +{ > + int i; > + u32 reg; > + > + arm_mmu500_reset(smmu); > + > + /* Re-enable context caching after reset */ > + for (i = 0; i < smmu->num_context_banks; ++i) { > + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > + reg |= 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); Is this applicable for sdm845? For all other platforms supported by qcom_smmu_500 implementation? > > /* > * To address performance degradation in non-real time clients, > @@ -509,7 +526,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, > }; > @@ -528,7 +545,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, > }; > @@ -544,7 +561,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-14 14:15 ` Dmitry Baryshkov @ 2023-11-15 9:45 ` Bibek Kumar Patro 2023-11-15 11:03 ` Dmitry Baryshkov 0 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-15 9:45 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> 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. > > Please refer to Documentation/process/submitting-patches.rst and > rephrase this following the rules there. > Noted, will go through the description once and rephrase it in next version complying with rules. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >> return match ? IOMMU_DOMAIN_IDENTITY : 0; >> } >> >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >> +{ >> + int i; >> + u32 reg; >> + >> + arm_mmu500_reset(smmu); >> + >> + /* Re-enable context caching after reset */ >> + for (i = 0; i < smmu->num_context_banks; ++i) { >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >> + reg |= 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); > > Is this applicable for sdm845? For all other platforms supported by > qcom_smmu_500 implementation? > In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c CPRE bit is reset for all SoC based on mmu500 platform, hence for all Qualcomm SoCs including sm845 we are setting back the CPRE bit. >> >> /* >> * To address performance degradation in non-real time clients, >> @@ -509,7 +526,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, >> }; >> @@ -528,7 +545,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, >> }; >> @@ -544,7 +561,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 >> > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-15 9:45 ` Bibek Kumar Patro @ 2023-11-15 11:03 ` Dmitry Baryshkov 2023-11-16 12:44 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-15 11:03 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > > On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> 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. > > > > Please refer to Documentation/process/submitting-patches.rst and > > rephrase this following the rules there. > > > > Noted, will go through the description once and rephrase it > in next version complying with rules. > > >> > >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >> --- > >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > >> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 > >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > >> return match ? IOMMU_DOMAIN_IDENTITY : 0; > >> } > >> > >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > >> +{ > >> + int i; > >> + u32 reg; > >> + > >> + arm_mmu500_reset(smmu); > >> + > >> + /* Re-enable context caching after reset */ > >> + for (i = 0; i < smmu->num_context_banks; ++i) { > >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > >> + reg |= 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); > > > > Is this applicable for sdm845? For all other platforms supported by > > qcom_smmu_500 implementation? > > > > In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > CPRE bit is reset for all SoC based on mmu500 platform, hence for all > Qualcomm SoCs including sm845 we are setting back the CPRE bit. The errata for the CoreLink MMU-500 requires CPRE to be disabled for all revisions before r2p2. Do we know whether these SoC used CoreLink MMU-500 and which version of it? > > >> > >> /* > >> * To address performance degradation in non-real time clients, > >> @@ -509,7 +526,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, > >> }; > >> @@ -528,7 +545,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, > >> }; > >> @@ -544,7 +561,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-15 11:03 ` Dmitry Baryshkov @ 2023-11-16 12:44 ` Bibek Kumar Patro 2023-11-16 15:24 ` Dmitry Baryshkov 0 siblings, 1 reply; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-16 12:44 UTC (permalink / raw) To: Dmitry Baryshkov Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: > On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> 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. >>> >>> Please refer to Documentation/process/submitting-patches.rst and >>> rephrase this following the rules there. >>> >> >> Noted, will go through the description once and rephrase it >> in next version complying with rules. >> >>>> >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >>>> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>> } >>>> >>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>> +{ >>>> + int i; >>>> + u32 reg; >>>> + >>>> + arm_mmu500_reset(smmu); >>>> + >>>> + /* Re-enable context caching after reset */ >>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>> + reg |= 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); >>> >>> Is this applicable for sdm845? For all other platforms supported by >>> qcom_smmu_500 implementation? >>> >> >> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >> Qualcomm SoCs including sm845 we are setting back the CPRE bit. > > The errata for the CoreLink MMU-500 requires CPRE to be disabled for > all revisions before r2p2. Do we know whether these SoC used CoreLink > MMU-500 and which version of it? > Just checked all these SoCs are using r2p4 revision. So CPRE needs to be enabled back here then? >> >>>> >>>> /* >>>> * To address performance degradation in non-real time clients, >>>> @@ -509,7 +526,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, >>>> }; >>>> @@ -528,7 +545,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, >>>> }; >>>> @@ -544,7 +561,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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-16 12:44 ` Bibek Kumar Patro @ 2023-11-16 15:24 ` Dmitry Baryshkov 2023-11-16 17:04 ` Robin Murphy 0 siblings, 1 reply; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-16 15:24 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: > > On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro > > <quic_bibekkum@quicinc.com> wrote: > >> > >> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: > >>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro > >>> <quic_bibekkum@quicinc.com> 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. > >>> > >>> Please refer to Documentation/process/submitting-patches.rst and > >>> rephrase this following the rules there. > >>> > >> > >> Noted, will go through the description once and rephrase it > >> in next version complying with rules. > >> > >>>> > >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >>>> --- > >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- > >>>> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 > >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) > >>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; > >>>> } > >>>> > >>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > >>>> +{ > >>>> + int i; > >>>> + u32 reg; > >>>> + > >>>> + arm_mmu500_reset(smmu); > >>>> + > >>>> + /* Re-enable context caching after reset */ > >>>> + for (i = 0; i < smmu->num_context_banks; ++i) { > >>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > >>>> + reg |= 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); > >>> > >>> Is this applicable for sdm845? For all other platforms supported by > >>> qcom_smmu_500 implementation? > >>> > >> > >> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c > >> CPRE bit is reset for all SoC based on mmu500 platform, hence for all > >> Qualcomm SoCs including sm845 we are setting back the CPRE bit. > > > > The errata for the CoreLink MMU-500 requires CPRE to be disabled for > > all revisions before r2p2. Do we know whether these SoC used CoreLink > > MMU-500 and which version of it? > > > > Just checked all these SoCs are using r2p4 revision. > So CPRE needs to be enabled back here then? can be enabled, yes. > > >> > >>>> > >>>> /* > >>>> * To address performance degradation in non-real time clients, > >>>> @@ -509,7 +526,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, > >>>> }; > >>>> @@ -528,7 +545,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, > >>>> }; > >>>> @@ -544,7 +561,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 -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-16 15:24 ` Dmitry Baryshkov @ 2023-11-16 17:04 ` Robin Murphy 2023-11-17 9:10 ` Bibek Kumar Patro 0 siblings, 1 reply; 34+ messages in thread From: Robin Murphy @ 2023-11-16 17:04 UTC (permalink / raw) To: Dmitry Baryshkov, Bibek Kumar Patro Cc: will, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 16/11/2023 3:24 pm, Dmitry Baryshkov wrote: > On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: >>> On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro >>> <quic_bibekkum@quicinc.com> wrote: >>>> >>>> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>> <quic_bibekkum@quicinc.com> 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. >>>>> >>>>> Please refer to Documentation/process/submitting-patches.rst and >>>>> rephrase this following the rules there. >>>>> >>>> >>>> Noted, will go through the description once and rephrase it >>>> in next version complying with rules. >>>> >>>>>> >>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>> --- >>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 ++++++++++++++++++---- >>>>>> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 >>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct device *dev) >>>>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>>>> } >>>>>> >>>>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>>>> +{ >>>>>> + int i; >>>>>> + u32 reg; >>>>>> + >>>>>> + arm_mmu500_reset(smmu); >>>>>> + >>>>>> + /* Re-enable context caching after reset */ >>>>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>>>> + reg |= 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); >>>>> >>>>> Is this applicable for sdm845? For all other platforms supported by >>>>> qcom_smmu_500 implementation? >>>>> >>>> >>>> In arm_mmu500_reset operation drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >>>> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >>>> Qualcomm SoCs including sm845 we are setting back the CPRE bit. >>> >>> The errata for the CoreLink MMU-500 requires CPRE to be disabled for >>> all revisions before r2p2. Do we know whether these SoC used CoreLink >>> MMU-500 and which version of it? >>> >> >> Just checked all these SoCs are using r2p4 revision. >> So CPRE needs to be enabled back here then? > > can be enabled, yes. There are still open errata #562869 and #1047329 which might need this workaround. I guess one could argue that we're not (knowingly) using nested translation at the moment, and also probably not running this in situations which would end up using short-descriptor format, however stuff like pKVM and IOMMUFD could potentially change those assumptions in future, so they still feel a bit sketchy to me. Thanks, Robin. > >> >>>> >>>>>> >>>>>> /* >>>>>> * To address performance degradation in non-real time clients, >>>>>> @@ -509,7 +526,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, >>>>>> }; >>>>>> @@ -528,7 +545,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, >>>>>> }; >>>>>> @@ -544,7 +561,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 > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-16 17:04 ` Robin Murphy @ 2023-11-17 9:10 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-17 9:10 UTC (permalink / raw) To: Robin Murphy, Dmitry Baryshkov Cc: will, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/16/2023 10:34 PM, Robin Murphy wrote: > On 16/11/2023 3:24 pm, Dmitry Baryshkov wrote: >> On Thu, 16 Nov 2023 at 14:45, Bibek Kumar Patro >> <quic_bibekkum@quicinc.com> wrote: >>> >>> >>> >>> On 11/15/2023 4:33 PM, Dmitry Baryshkov wrote: >>>> On Wed, 15 Nov 2023 at 11:45, Bibek Kumar Patro >>>> <quic_bibekkum@quicinc.com> wrote: >>>>> >>>>> On 11/14/2023 7:45 PM, Dmitry Baryshkov wrote: >>>>>> On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro >>>>>> <quic_bibekkum@quicinc.com> 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. >>>>>> >>>>>> Please refer to Documentation/process/submitting-patches.rst and >>>>>> rephrase this following the rules there. >>>>>> >>>>> >>>>> Noted, will go through the description once and rephrase it >>>>> in next version complying with rules. >>>>> >>>>>>> >>>>>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>>>>> --- >>>>>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 25 >>>>>>> ++++++++++++++++++---- >>>>>>> 1 file changed, 21 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 0eaf6f2a2e49..fa867b1d9d16 100644 >>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>>>>> @@ -478,11 +478,28 @@ static int qcom_smmu_def_domain_type(struct >>>>>>> device *dev) >>>>>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>>>>> } >>>>>>> >>>>>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>>>>> +{ >>>>>>> + int i; >>>>>>> + u32 reg; >>>>>>> + >>>>>>> + arm_mmu500_reset(smmu); >>>>>>> + >>>>>>> + /* Re-enable context caching after reset */ >>>>>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>>>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>>>>> + reg |= 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); >>>>>> >>>>>> Is this applicable for sdm845? For all other platforms supported by >>>>>> qcom_smmu_500 implementation? >>>>>> >>>>> >>>>> In arm_mmu500_reset operation >>>>> drivers/iommu/arm/arm-smmu/arm-smmu-impl.c >>>>> CPRE bit is reset for all SoC based on mmu500 platform, hence for all >>>>> Qualcomm SoCs including sm845 we are setting back the CPRE bit. >>>> >>>> The errata for the CoreLink MMU-500 requires CPRE to be disabled for >>>> all revisions before r2p2. Do we know whether these SoC used CoreLink >>>> MMU-500 and which version of it? >>>> >>> >>> Just checked all these SoCs are using r2p4 revision. >>> So CPRE needs to be enabled back here then? >> >> can be enabled, yes. > > There are still open errata #562869 and #1047329 which might need this > workaround. I guess one could argue that we're not (knowingly) using > nested translation at the moment, and also probably not running this in > situations which would end up using short-descriptor format, however > stuff like pKVM and IOMMUFD could potentially change those assumptions > in future, so they still feel a bit sketchy to me. > Could you help provide some details on these two errata (#562869 and #1047329).Both of these erratum are there for r2p4 revisions as well? Thanks & regards, Bibek > Thanks, > Robin. > >> >>> >>>>> >>>>>>> >>>>>>> /* >>>>>>> * To address performance degradation in non-real time >>>>>>> clients, >>>>>>> @@ -509,7 +526,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, >>>>>>> }; >>>>>>> @@ -528,7 +545,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, >>>>>>> }; >>>>>>> @@ -544,7 +561,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 >> >> >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-14 13:56 ` [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation Bibek Kumar Patro 2023-11-14 14:15 ` Dmitry Baryshkov @ 2023-11-15 16:43 ` Konrad Dybcio 2023-11-17 7:15 ` Bibek Kumar Patro 1 sibling, 1 reply; 34+ messages in thread From: Konrad Dybcio @ 2023-11-15 16:43 UTC (permalink / raw) To: Bibek Kumar Patro, will, robin.murphy, joro, dmitry.baryshkov, a39.skl, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/14/23 14:56, 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 <quic_bibekkum@quicinc.com> > --- And I assume that goes for all SMMU500 implementations? Looking at the 8550 ACTRL array from patch 2, CPRE is not enabled at all times.. Is that because of performance, or some other technical reason? Will this regress platforms without ACTRL tables? Konrad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation 2023-11-15 16:43 ` Konrad Dybcio @ 2023-11-17 7:15 ` Bibek Kumar Patro 0 siblings, 0 replies; 34+ messages in thread From: Bibek Kumar Patro @ 2023-11-17 7:15 UTC (permalink / raw) To: Konrad Dybcio, will, robin.murphy, joro, dmitry.baryshkov, a39.skl, quic_pkondeti, quic_molvera Cc: linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On 11/15/2023 10:13 PM, Konrad Dybcio wrote: > > > On 11/14/23 14:56, 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 <quic_bibekkum@quicinc.com> >> --- > And I assume that goes for all SMMU500 implementations? > Right, for all SMMU500 implementation for Qualcomm SoCs. Hence implemented this enablement with Qualcomm specific reset operation. > Looking at the 8550 ACTRL array from patch 2, CPRE is not enabled > at all times.. Is that because of performance, or some other > technical reason? > > Will this regress platforms without ACTRL tables? > It should not regress, If you check my recent reply on Dimitry's response, the Corelink revision is r2p4 and it can be enabled. On the Robin's mentioned errata workarounds, let me check once. Thanks & regards, Bibek > Konrad _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs 2023-11-14 13:56 [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Bibek Kumar Patro ` (2 preceding siblings ...) 2023-11-14 13:56 ` [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation Bibek Kumar Patro @ 2023-11-14 14:06 ` Dmitry Baryshkov 3 siblings, 0 replies; 34+ messages in thread From: Dmitry Baryshkov @ 2023-11-14 14:06 UTC (permalink / raw) To: Bibek Kumar Patro Cc: will, robin.murphy, joro, a39.skl, konrad.dybcio, quic_pkondeti, quic_molvera, linux-arm-msm, linux-arm-kernel, iommu, linux-kernel, qipl.kernel.upstream On Tue, 14 Nov 2023 at 15:57, Bibek Kumar Patro <quic_bibekkum@quicinc.com> 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. > > Changes in v2 from v1: > - Incorporated suggestions on v1 from Dmitry,Konrad,Pratyush. Which suggestions? This doesn't tell anything. > - Added defines for ACTLR values. > - Linked sm8550 implementation structure to corresponding > compatible string. > - Repackaged actlr value set implementation to separate function. > - Fixed indentation errors. > - Link to v1: https://lore.kernel.org/all/20231103215124.1095-1-quic_bibekkum@quicinc.com/ > > Changes in v1 from RFC: > - Incorporated suggestion form Robin on RFC. > - Moved the actlr data table into driver, instead of maintaining > it inside soc specific DT and piggybacking on exisiting iommus > property (iommu = <SID, MASK, ACTLR>) to set this value during > smmu probe. > - Link to RFC: //lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/ > > 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 | 156 ++++++++++++++++++++- > 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, 159 insertions(+), 9 deletions(-) > > -- > 2.17.1 > -- With best wishes Dmitry _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2023-11-20 10:26 UTC | newest] Thread overview: 34+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-11-14 13:56 [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings Bibek Kumar Patro 2023-11-14 14:08 ` Dmitry Baryshkov 2023-11-15 5:42 ` Bibek Kumar Patro 2023-11-14 16:55 ` Robin Murphy 2023-11-15 12:32 ` Bibek Kumar Patro 2023-11-15 15:04 ` Robin Murphy 2023-11-20 10:24 ` Bibek Kumar Patro 2023-11-15 13:54 ` Bibek Kumar Patro 2023-11-15 14:53 ` Robin Murphy 2023-11-17 5:27 ` Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550 Bibek Kumar Patro 2023-11-14 14:12 ` Dmitry Baryshkov 2023-11-15 9:22 ` Bibek Kumar Patro 2023-11-15 9:38 ` Dmitry Baryshkov 2023-11-15 9:51 ` Bibek Kumar Patro 2023-11-15 10:45 ` Dmitry Baryshkov 2023-11-15 12:49 ` Bibek Kumar Patro 2023-11-15 13:10 ` Dmitry Baryshkov 2023-11-15 16:42 ` Konrad Dybcio 2023-11-16 6:09 ` Bibek Kumar Patro 2023-11-16 6:51 ` Dmitry Baryshkov 2023-11-17 5:36 ` Bibek Kumar Patro 2023-11-14 13:56 ` [PATCH v2 3/3] iommu/arm-smmu: re-enable context caching in smmu reset operation Bibek Kumar Patro 2023-11-14 14:15 ` Dmitry Baryshkov 2023-11-15 9:45 ` Bibek Kumar Patro 2023-11-15 11:03 ` Dmitry Baryshkov 2023-11-16 12:44 ` Bibek Kumar Patro 2023-11-16 15:24 ` Dmitry Baryshkov 2023-11-16 17:04 ` Robin Murphy 2023-11-17 9:10 ` Bibek Kumar Patro 2023-11-15 16:43 ` Konrad Dybcio 2023-11-17 7:15 ` Bibek Kumar Patro 2023-11-14 14:06 ` [PATCH v2 0/3] iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs Dmitry Baryshkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox