| Message ID | 20231114135654.30475-3-quic_bibekkum@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs | expand |
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 >
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 >> > >
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 > >> > > > >
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 >>>> >>> >>> > > >
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 > >>>> > >>> > >>> > > > > > >
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 >>>>>> >>>>> >>>>> >>> >>> >>> > > >
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 > >>>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > >
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
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
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.
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.
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 }, { } };
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(-) -- 2.17.1