| Message ID | 20231103215124.1095-4-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 Fri, 3 Nov 2023 at 23:53, 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. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 590b7c285299..f342b4778cf1 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) > return match ? IOMMU_DOMAIN_IDENTITY : 0; > } > > +#define ARM_MMU500_ACTLR_CPRE BIT(1) > + > +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > +{ > + int i; > + u32 reg; > + > + arm_mmu500_reset(smmu); > + > + for (i = 0; i < smmu->num_context_banks; ++i) { > + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > + reg |= ARM_MMU500_ACTLR_CPRE; > + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); > + } Wrong indentation. Did you run your patches through checkpatch.pl? > + > + return 0; > +} > + > static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > { > int ret; > > - arm_mmu500_reset(smmu); > + qcom_smmu500_reset(smmu); > > /* > * To address performance degradation in non-real time clients, > @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { > .init_context = qcom_smmu_init_context, > .cfg_probe = qcom_smmu_cfg_probe, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .write_s2cr = qcom_smmu_write_s2cr, > .tlb_sync = qcom_smmu_tlb_sync, > }; > @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { > .init_context = qcom_smmu_init_context, > .cfg_probe = qcom_smmu_cfg_probe, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .write_s2cr = qcom_smmu_write_s2cr, > .tlb_sync = qcom_smmu_tlb_sync, > }; > @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { > static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { > .init_context = qcom_adreno_smmu_init_context, > .def_domain_type = qcom_smmu_def_domain_type, > - .reset = arm_mmu500_reset, > + .reset = qcom_smmu500_reset, > .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > .write_sctlr = qcom_adreno_smmu_write_sctlr, > .tlb_sync = qcom_smmu_tlb_sync, > -- > 2.17.1 >
On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote: > On Fri, 3 Nov 2023 at 23:53, 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. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- >> 1 file changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> index 590b7c285299..f342b4778cf1 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) >> return match ? IOMMU_DOMAIN_IDENTITY : 0; >> } >> >> +#define ARM_MMU500_ACTLR_CPRE BIT(1) >> + >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >> +{ >> + int i; >> + u32 reg; >> + >> + arm_mmu500_reset(smmu); >> + >> + for (i = 0; i < smmu->num_context_banks; ++i) { >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >> + reg |= ARM_MMU500_ACTLR_CPRE; >> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >> + } > > Wrong indentation. Did you run your patches through checkpatch.pl? > Yes Dmitry, I did run checkpatch.pl script on this patch as well as others, got 0 errors and 0 warnings. With -f option as well. Did not get any related errors and warnings. >> + >> + return 0; >> +} >> + >> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) >> { >> int ret; >> >> - arm_mmu500_reset(smmu); >> + qcom_smmu500_reset(smmu); >> >> /* >> * To address performance degradation in non-real time clients, >> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { >> .init_context = qcom_smmu_init_context, >> .cfg_probe = qcom_smmu_cfg_probe, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .write_s2cr = qcom_smmu_write_s2cr, >> .tlb_sync = qcom_smmu_tlb_sync, >> }; >> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { >> .init_context = qcom_smmu_init_context, >> .cfg_probe = qcom_smmu_cfg_probe, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .write_s2cr = qcom_smmu_write_s2cr, >> .tlb_sync = qcom_smmu_tlb_sync, >> }; >> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { >> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >> .init_context = qcom_adreno_smmu_init_context, >> .def_domain_type = qcom_smmu_def_domain_type, >> - .reset = arm_mmu500_reset, >> + .reset = qcom_smmu500_reset, >> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, >> .write_sctlr = qcom_adreno_smmu_write_sctlr, >> .tlb_sync = qcom_smmu_tlb_sync, >> -- >> 2.17.1 >> > >
On Sat, 4 Nov 2023 at 00:07, Bibek Kumar Patro <quic_bibekkum@quicinc.com> wrote: > > > > On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote: > > On Fri, 3 Nov 2023 at 23:53, 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. > >> > >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > >> --- > >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- > >> 1 file changed, 22 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> index 590b7c285299..f342b4778cf1 100644 > >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > >> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) > >> return match ? IOMMU_DOMAIN_IDENTITY : 0; > >> } > >> > >> +#define ARM_MMU500_ACTLR_CPRE BIT(1) > >> + > >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > >> +{ > >> + int i; > >> + u32 reg; > >> + > >> + arm_mmu500_reset(smmu); > >> + > >> + for (i = 0; i < smmu->num_context_banks; ++i) { > >> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); > >> + reg |= ARM_MMU500_ACTLR_CPRE; > >> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); > >> + } > > > > Wrong indentation. Did you run your patches through checkpatch.pl? > > > > Yes Dmitry, I did run checkpatch.pl script on this patch as well as > others, got 0 errors and 0 warnings. With -f option as well. Did not > get any related errors and warnings. Ack, I beg your pardon. checkpatch indeed doesn't warn about this indentation. Though it is still incorrect. > > >> + > >> + return 0; > >> +} > >> + > >> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) > >> { > >> int ret; > >> > >> - arm_mmu500_reset(smmu); > >> + qcom_smmu500_reset(smmu); > >> > >> /* > >> * To address performance degradation in non-real time clients, > >> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { > >> .init_context = qcom_smmu_init_context, > >> .cfg_probe = qcom_smmu_cfg_probe, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .write_s2cr = qcom_smmu_write_s2cr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> }; > >> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { > >> .init_context = qcom_smmu_init_context, > >> .cfg_probe = qcom_smmu_cfg_probe, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .write_s2cr = qcom_smmu_write_s2cr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> }; > >> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { > >> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { > >> .init_context = qcom_adreno_smmu_init_context, > >> .def_domain_type = qcom_smmu_def_domain_type, > >> - .reset = arm_mmu500_reset, > >> + .reset = qcom_smmu500_reset, > >> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, > >> .write_sctlr = qcom_adreno_smmu_write_sctlr, > >> .tlb_sync = qcom_smmu_tlb_sync, > >> -- > >> 2.17.1 > >> > > > >
On 11/4/2023 3:53 AM, Dmitry Baryshkov wrote: > On Sat, 4 Nov 2023 at 00:07, Bibek Kumar Patro > <quic_bibekkum@quicinc.com> wrote: >> >> >> >> On 11/4/2023 3:28 AM, Dmitry Baryshkov wrote: >>> On Fri, 3 Nov 2023 at 23:53, 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. >>>> >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- >>>> 1 file changed, 22 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> index 590b7c285299..f342b4778cf1 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) >>>> return match ? IOMMU_DOMAIN_IDENTITY : 0; >>>> } >>>> >>>> +#define ARM_MMU500_ACTLR_CPRE BIT(1) >>>> + >>>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >>>> +{ >>>> + int i; >>>> + u32 reg; >>>> + >>>> + arm_mmu500_reset(smmu); >>>> + >>>> + for (i = 0; i < smmu->num_context_banks; ++i) { >>>> + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); >>>> + reg |= ARM_MMU500_ACTLR_CPRE; >>>> + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); >>>> + } >>> >>> Wrong indentation. Did you run your patches through checkpatch.pl? >>> >> >> Yes Dmitry, I did run checkpatch.pl script on this patch as well as >> others, got 0 errors and 0 warnings. With -f option as well. Did not >> get any related errors and warnings. > > Ack, I beg your pardon. checkpatch indeed doesn't warn about this > indentation. Though it is still incorrect. > Ack, thanks for pointing this out. Will fix this indentation for loop in next iteration. >> >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) >>>> { >>>> int ret; >>>> >>>> - arm_mmu500_reset(smmu); >>>> + qcom_smmu500_reset(smmu); >>>> >>>> /* >>>> * To address performance degradation in non-real time clients, >>>> @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { >>>> .init_context = qcom_smmu_init_context, >>>> .cfg_probe = qcom_smmu_cfg_probe, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .write_s2cr = qcom_smmu_write_s2cr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> }; >>>> @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { >>>> .init_context = qcom_smmu_init_context, >>>> .cfg_probe = qcom_smmu_cfg_probe, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .write_s2cr = qcom_smmu_write_s2cr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> }; >>>> @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { >>>> static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { >>>> .init_context = qcom_adreno_smmu_init_context, >>>> .def_domain_type = qcom_smmu_def_domain_type, >>>> - .reset = arm_mmu500_reset, >>>> + .reset = qcom_smmu500_reset, >>>> .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, >>>> .write_sctlr = qcom_adreno_smmu_write_sctlr, >>>> .tlb_sync = qcom_smmu_tlb_sync, >>>> -- >>>> 2.17.1 >>>> >>> >>> > > >
On 11/3/23 22:51, Bibek Kumar Patro wrote: > Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs > through SoC specific reset ops, which is disabled in the default MMU-500 > reset ops, but is expected for context banks using ACTLR register to > retain the prefetch value during reset and runtime suspend. > > Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 590b7c285299..f342b4778cf1 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) > return match ? IOMMU_DOMAIN_IDENTITY : 0; > } > > +#define ARM_MMU500_ACTLR_CPRE BIT(1) > + > +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) > +{ > + int i; > + u32 reg; > + > + arm_mmu500_reset(smmu); > + > + for (i = 0; i < smmu->num_context_banks; ++i) { This loop deserves a comment above it like /* Re-enable context caching after reset */ Konrad
On 11/4/2023 5:00 PM, Konrad Dybcio wrote: > > > On 11/3/23 22:51, Bibek Kumar Patro wrote: >> Context caching is re-enabled in the prefetch buffer for Qualcomm SoCs >> through SoC specific reset ops, which is disabled in the default MMU-500 >> reset ops, but is expected for context banks using ACTLR register to >> retain the prefetch value during reset and runtime suspend. >> >> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com> >> --- >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 26 ++++++++++++++++++---- >> 1 file changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> index 590b7c285299..f342b4778cf1 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct >> device *dev) >> return match ? IOMMU_DOMAIN_IDENTITY : 0; >> } >> >> +#define ARM_MMU500_ACTLR_CPRE BIT(1) >> + >> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) >> +{ >> + int i; >> + u32 reg; >> + >> + arm_mmu500_reset(smmu); >> + >> + for (i = 0; i < smmu->num_context_banks; ++i) { > This loop deserves a comment above it like > > /* Re-enable context caching after reset */ > Ack, thanks for this suggestion, this might help to explain this addition. I would incorporate this in next patch. regards, Bibek > Konrad
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 590b7c285299..f342b4778cf1 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -457,11 +457,29 @@ static int qcom_smmu_def_domain_type(struct device *dev) return match ? IOMMU_DOMAIN_IDENTITY : 0; } +#define ARM_MMU500_ACTLR_CPRE BIT(1) + +static int qcom_smmu500_reset(struct arm_smmu_device *smmu) +{ + int i; + u32 reg; + + arm_mmu500_reset(smmu); + + for (i = 0; i < smmu->num_context_banks; ++i) { + reg = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR); + reg |= ARM_MMU500_ACTLR_CPRE; + arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, reg); + } + + return 0; +} + static int qcom_sdm845_smmu500_reset(struct arm_smmu_device *smmu) { int ret; - arm_mmu500_reset(smmu); + qcom_smmu500_reset(smmu); /* * To address performance degradation in non-real time clients, @@ -488,7 +506,7 @@ static const struct arm_smmu_impl qcom_smmu_500_impl = { .init_context = qcom_smmu_init_context, .cfg_probe = qcom_smmu_cfg_probe, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .write_s2cr = qcom_smmu_write_s2cr, .tlb_sync = qcom_smmu_tlb_sync, }; @@ -507,7 +525,7 @@ static const struct arm_smmu_impl sm8550_smmu_500_impl = { .init_context = qcom_smmu_init_context, .cfg_probe = qcom_smmu_cfg_probe, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .write_s2cr = qcom_smmu_write_s2cr, .tlb_sync = qcom_smmu_tlb_sync, }; @@ -523,7 +541,7 @@ static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = { static const struct arm_smmu_impl qcom_adreno_smmu_500_impl = { .init_context = qcom_adreno_smmu_init_context, .def_domain_type = qcom_smmu_def_domain_type, - .reset = arm_mmu500_reset, + .reset = qcom_smmu500_reset, .alloc_context_bank = qcom_adreno_smmu_alloc_context_bank, .write_sctlr = qcom_adreno_smmu_write_sctlr, .tlb_sync = qcom_smmu_tlb_sync,
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 | 26 ++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) -- 2.17.1