diff mbox series

[2/3] iommu/arm-smmu: add ACTLR data and support for SM8550

Message ID 20231103215124.1095-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

Commit Message

Bibek Kumar Patro Nov. 3, 2023, 9:51 p.m. UTC
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 | 85 +++++++++++++++++++++-
 1 file changed, 81 insertions(+), 4 deletions(-)

--
2.17.1

Comments

Dmitry Baryshkov Nov. 3, 2023, 10:01 p.m. UTC | #1
On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
>  1 file changed, 81 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 68c1f4908473..590b7c285299 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -25,6 +25,64 @@ struct actlr_data {
>         u32 actlr;
>  };
>
> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> +       { 0x18a0, 0x0000, 0x00000103 },
> +       { 0x18e0, 0x0000, 0x00000103 },
> +       { 0x0800, 0x0020, 0x00000001 },
> +       { 0x1800, 0x00c0, 0x00000001 },
> +       { 0x1820, 0x0000, 0x00000001 },
> +       { 0x1860, 0x0000, 0x00000001 },
> +       { 0x0c01, 0x0020, 0x00000303 },
> +       { 0x0c02, 0x0020, 0x00000303 },
> +       { 0x0c03, 0x0020, 0x00000303 },
> +       { 0x0c04, 0x0020, 0x00000303 },
> +       { 0x0c05, 0x0020, 0x00000303 },
> +       { 0x0c06, 0x0020, 0x00000303 },
> +       { 0x0c07, 0x0020, 0x00000303 },
> +       { 0x0c08, 0x0020, 0x00000303 },
> +       { 0x0c09, 0x0020, 0x00000303 },
> +       { 0x0c0c, 0x0020, 0x00000303 },
> +       { 0x0c0d, 0x0020, 0x00000303 },
> +       { 0x0c0e, 0x0020, 0x00000303 },
> +       { 0x0c0f, 0x0020, 0x00000303 },
> +       { 0x1961, 0x0000, 0x00000303 },
> +       { 0x1962, 0x0000, 0x00000303 },
> +       { 0x1963, 0x0000, 0x00000303 },
> +       { 0x1964, 0x0000, 0x00000303 },
> +       { 0x1965, 0x0000, 0x00000303 },
> +       { 0x1966, 0x0000, 0x00000303 },
> +       { 0x1967, 0x0000, 0x00000303 },
> +       { 0x1968, 0x0000, 0x00000303 },
> +       { 0x1969, 0x0000, 0x00000303 },
> +       { 0x196c, 0x0000, 0x00000303 },
> +       { 0x196d, 0x0000, 0x00000303 },
> +       { 0x196e, 0x0000, 0x00000303 },
> +       { 0x196f, 0x0000, 0x00000303 },
> +       { 0x19c1, 0x0010, 0x00000303 },
> +       { 0x19c2, 0x0010, 0x00000303 },
> +       { 0x19c3, 0x0010, 0x00000303 },
> +       { 0x19c4, 0x0010, 0x00000303 },
> +       { 0x19c5, 0x0010, 0x00000303 },
> +       { 0x19c6, 0x0010, 0x00000303 },
> +       { 0x19c7, 0x0010, 0x00000303 },
> +       { 0x19c8, 0x0010, 0x00000303 },
> +       { 0x19c9, 0x0010, 0x00000303 },
> +       { 0x19cc, 0x0010, 0x00000303 },
> +       { 0x19cd, 0x0010, 0x00000303 },
> +       { 0x19ce, 0x0010, 0x00000303 },
> +       { 0x19cf, 0x0010, 0x00000303 },
> +       { 0x1c00, 0x0002, 0x00000103 },
> +       { 0x1c01, 0x0000, 0x00000001 },
> +       { 0x1920, 0x0000, 0x00000103 },
> +       { 0x1923, 0x0000, 0x00000103 },
> +       { 0x1924, 0x0000, 0x00000103 },
> +       { 0x1940, 0x0000, 0x00000103 },
> +       { 0x1941, 0x0004, 0x00000103 },
> +       { 0x1943, 0x0000, 0x00000103 },
> +       { 0x1944, 0x0000, 0x00000103 },
> +       { 0x1947, 0x0000, 0x00000103 },
> +};

This is nearly impossible to handle.
Please add defines for 0x1, 0x103 and 0x303. Also please consider
adding comments for the devices.

> +
>  static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>  {
>         return container_of(smmu, struct qcom_smmu, smmu);
> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>         .tlb_sync = qcom_smmu_tlb_sync,
>  };
>
> +
> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> +       .init_context = qcom_smmu_init_context,
> +       .cfg_probe = qcom_smmu_cfg_probe,
> +       .def_domain_type = qcom_smmu_def_domain_type,
> +       .reset = arm_mmu500_reset,
> +       .write_s2cr = qcom_smmu_write_s2cr,
> +       .tlb_sync = qcom_smmu_tlb_sync,
> +};
> +
>  static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>         .init_context = qcom_adreno_smmu_init_context,
>         .def_domain_type = qcom_smmu_def_domain_type,
> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>         .reg_offset = qcom_smmu_impl0_reg_offset,
>  };
>
> +static const struct actlr_config sm8550_actlrcfg = {
> +       .adata = sm8550_apps_actlr_data,
> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> +};
> +
>  /*
>   * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>   * there are not enough context banks.
> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>         /* Also no debug configuration. */
>  };
>
> +
> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> +       .impl = &sm8550_smmu_500_impl,
> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
> +       .cfg = &qcom_smmu_impl0_cfg,
> +       .actlrcfg = &sm8550_actlrcfg,
> +};

This structure doesn't seem to be linked. Did you test your patches?

> +
>  static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>         .impl = &qcom_smmu_500_impl,
>         .adreno_impl = &qcom_adreno_smmu_500_impl,
>         .cfg = &qcom_smmu_impl0_cfg,
>  };
>
> -/*
> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> - * special handling and can not be covered by the qcom,smmu-500 entry.
> - */

Leave the comment in place.

>  static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>         { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>         { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> --
> 2.17.1
>
Bibek Kumar Patro Nov. 3, 2023, 10:38 p.m. UTC | #2
On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
>>   1 file changed, 81 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 68c1f4908473..590b7c285299 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -25,6 +25,64 @@ struct actlr_data {
>>          u32 actlr;
>>   };
>>
>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>> +       { 0x18a0, 0x0000, 0x00000103 },
>> +       { 0x18e0, 0x0000, 0x00000103 },
>> +       { 0x0800, 0x0020, 0x00000001 },
>> +       { 0x1800, 0x00c0, 0x00000001 },
>> +       { 0x1820, 0x0000, 0x00000001 },
>> +       { 0x1860, 0x0000, 0x00000001 },
>> +       { 0x0c01, 0x0020, 0x00000303 },
>> +       { 0x0c02, 0x0020, 0x00000303 },
>> +       { 0x0c03, 0x0020, 0x00000303 },
>> +       { 0x0c04, 0x0020, 0x00000303 },
>> +       { 0x0c05, 0x0020, 0x00000303 },
>> +       { 0x0c06, 0x0020, 0x00000303 },
>> +       { 0x0c07, 0x0020, 0x00000303 },
>> +       { 0x0c08, 0x0020, 0x00000303 },
>> +       { 0x0c09, 0x0020, 0x00000303 },
>> +       { 0x0c0c, 0x0020, 0x00000303 },
>> +       { 0x0c0d, 0x0020, 0x00000303 },
>> +       { 0x0c0e, 0x0020, 0x00000303 },
>> +       { 0x0c0f, 0x0020, 0x00000303 },
>> +       { 0x1961, 0x0000, 0x00000303 },
>> +       { 0x1962, 0x0000, 0x00000303 },
>> +       { 0x1963, 0x0000, 0x00000303 },
>> +       { 0x1964, 0x0000, 0x00000303 },
>> +       { 0x1965, 0x0000, 0x00000303 },
>> +       { 0x1966, 0x0000, 0x00000303 },
>> +       { 0x1967, 0x0000, 0x00000303 },
>> +       { 0x1968, 0x0000, 0x00000303 },
>> +       { 0x1969, 0x0000, 0x00000303 },
>> +       { 0x196c, 0x0000, 0x00000303 },
>> +       { 0x196d, 0x0000, 0x00000303 },
>> +       { 0x196e, 0x0000, 0x00000303 },
>> +       { 0x196f, 0x0000, 0x00000303 },
>> +       { 0x19c1, 0x0010, 0x00000303 },
>> +       { 0x19c2, 0x0010, 0x00000303 },
>> +       { 0x19c3, 0x0010, 0x00000303 },
>> +       { 0x19c4, 0x0010, 0x00000303 },
>> +       { 0x19c5, 0x0010, 0x00000303 },
>> +       { 0x19c6, 0x0010, 0x00000303 },
>> +       { 0x19c7, 0x0010, 0x00000303 },
>> +       { 0x19c8, 0x0010, 0x00000303 },
>> +       { 0x19c9, 0x0010, 0x00000303 },
>> +       { 0x19cc, 0x0010, 0x00000303 },
>> +       { 0x19cd, 0x0010, 0x00000303 },
>> +       { 0x19ce, 0x0010, 0x00000303 },
>> +       { 0x19cf, 0x0010, 0x00000303 },
>> +       { 0x1c00, 0x0002, 0x00000103 },
>> +       { 0x1c01, 0x0000, 0x00000001 },
>> +       { 0x1920, 0x0000, 0x00000103 },
>> +       { 0x1923, 0x0000, 0x00000103 },
>> +       { 0x1924, 0x0000, 0x00000103 },
>> +       { 0x1940, 0x0000, 0x00000103 },
>> +       { 0x1941, 0x0004, 0x00000103 },
>> +       { 0x1943, 0x0000, 0x00000103 },
>> +       { 0x1944, 0x0000, 0x00000103 },
>> +       { 0x1947, 0x0000, 0x00000103 },
>> +};
> 
> This is nearly impossible to handle.
> Please add defines for 0x1, 0x103 and 0x303. Also please consider
> adding comments for the devices.
> 

Ack, Initially added the comments for devices, but since only driver is
handling this data, and clients won't refer this so removed it. Will
consider adding it back. This actlr field value might different (other
than 0x1,0x103,0x303) in other platforms as per my anticipation, 
depending on the bit settings, so won't the defines change with 
different platforms? Hence this register setting value might be apt?

>> +
>>   static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>   {
>>          return container_of(smmu, struct qcom_smmu, smmu);
>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>>          .tlb_sync = qcom_smmu_tlb_sync,
>>   };
>>
>> +
>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>> +       .init_context = qcom_smmu_init_context,
>> +       .cfg_probe = qcom_smmu_cfg_probe,
>> +       .def_domain_type = qcom_smmu_def_domain_type,
>> +       .reset = arm_mmu500_reset,
>> +       .write_s2cr = qcom_smmu_write_s2cr,
>> +       .tlb_sync = qcom_smmu_tlb_sync,
>> +};
>> +
>>   static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>          .init_context = qcom_adreno_smmu_init_context,
>>          .def_domain_type = qcom_smmu_def_domain_type,
>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>>          .reg_offset = qcom_smmu_impl0_reg_offset,
>>   };
>>
>> +static const struct actlr_config sm8550_actlrcfg = {
>> +       .adata = sm8550_apps_actlr_data,
>> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>> +};
>> +
>>   /*
>>    * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>>    * there are not enough context banks.
>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>>          /* Also no debug configuration. */
>>   };
>>
>> +
>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>> +       .impl = &sm8550_smmu_500_impl,
>> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
>> +       .cfg = &qcom_smmu_impl0_cfg,
>> +       .actlrcfg = &sm8550_actlrcfg,
>> +};
> 
> This structure doesn't seem to be linked. Did you test your patches?
> 

Apologies Dmitry, just checked back my patches, I tested it but while 
refining the patches I somehow missed this link
{ .compatible = "qcom,sm8550-smmu-500", .data = 
&sm8550_smmu_500_impl0_data };
in below qcom_smmu_500_impl0_data structure.
I will take care of this in next version.

>> +
>>   static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>>          .impl = &qcom_smmu_500_impl,
>>          .adreno_impl = &qcom_adreno_smmu_500_impl,
>>          .cfg = &qcom_smmu_impl0_cfg,
>>   };
>>
>> -/*
>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>> - */
> 
> Leave the comment in place.
> 

Thanks for this comment which helped me to note the above mentioned
mistake.

>>   static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>>          { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>>          { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>> --
>> 2.17.1
>>
> 
>
Dmitry Baryshkov Nov. 3, 2023, 10:41 p.m. UTC | #3
On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
<quic_bibekkum@quicinc.com> wrote:
>
>
>
> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> > On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
> >>   1 file changed, 81 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> index 68c1f4908473..590b7c285299 100644
> >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >> @@ -25,6 +25,64 @@ struct actlr_data {
> >>          u32 actlr;
> >>   };
> >>
> >> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> >> +       { 0x18a0, 0x0000, 0x00000103 },
> >> +       { 0x18e0, 0x0000, 0x00000103 },
> >> +       { 0x0800, 0x0020, 0x00000001 },
> >> +       { 0x1800, 0x00c0, 0x00000001 },
> >> +       { 0x1820, 0x0000, 0x00000001 },
> >> +       { 0x1860, 0x0000, 0x00000001 },
> >> +       { 0x0c01, 0x0020, 0x00000303 },
> >> +       { 0x0c02, 0x0020, 0x00000303 },
> >> +       { 0x0c03, 0x0020, 0x00000303 },
> >> +       { 0x0c04, 0x0020, 0x00000303 },
> >> +       { 0x0c05, 0x0020, 0x00000303 },
> >> +       { 0x0c06, 0x0020, 0x00000303 },
> >> +       { 0x0c07, 0x0020, 0x00000303 },
> >> +       { 0x0c08, 0x0020, 0x00000303 },
> >> +       { 0x0c09, 0x0020, 0x00000303 },
> >> +       { 0x0c0c, 0x0020, 0x00000303 },
> >> +       { 0x0c0d, 0x0020, 0x00000303 },
> >> +       { 0x0c0e, 0x0020, 0x00000303 },
> >> +       { 0x0c0f, 0x0020, 0x00000303 },
> >> +       { 0x1961, 0x0000, 0x00000303 },
> >> +       { 0x1962, 0x0000, 0x00000303 },
> >> +       { 0x1963, 0x0000, 0x00000303 },
> >> +       { 0x1964, 0x0000, 0x00000303 },
> >> +       { 0x1965, 0x0000, 0x00000303 },
> >> +       { 0x1966, 0x0000, 0x00000303 },
> >> +       { 0x1967, 0x0000, 0x00000303 },
> >> +       { 0x1968, 0x0000, 0x00000303 },
> >> +       { 0x1969, 0x0000, 0x00000303 },
> >> +       { 0x196c, 0x0000, 0x00000303 },
> >> +       { 0x196d, 0x0000, 0x00000303 },
> >> +       { 0x196e, 0x0000, 0x00000303 },
> >> +       { 0x196f, 0x0000, 0x00000303 },
> >> +       { 0x19c1, 0x0010, 0x00000303 },
> >> +       { 0x19c2, 0x0010, 0x00000303 },
> >> +       { 0x19c3, 0x0010, 0x00000303 },
> >> +       { 0x19c4, 0x0010, 0x00000303 },
> >> +       { 0x19c5, 0x0010, 0x00000303 },
> >> +       { 0x19c6, 0x0010, 0x00000303 },
> >> +       { 0x19c7, 0x0010, 0x00000303 },
> >> +       { 0x19c8, 0x0010, 0x00000303 },
> >> +       { 0x19c9, 0x0010, 0x00000303 },
> >> +       { 0x19cc, 0x0010, 0x00000303 },
> >> +       { 0x19cd, 0x0010, 0x00000303 },
> >> +       { 0x19ce, 0x0010, 0x00000303 },
> >> +       { 0x19cf, 0x0010, 0x00000303 },
> >> +       { 0x1c00, 0x0002, 0x00000103 },
> >> +       { 0x1c01, 0x0000, 0x00000001 },
> >> +       { 0x1920, 0x0000, 0x00000103 },
> >> +       { 0x1923, 0x0000, 0x00000103 },
> >> +       { 0x1924, 0x0000, 0x00000103 },
> >> +       { 0x1940, 0x0000, 0x00000103 },
> >> +       { 0x1941, 0x0004, 0x00000103 },
> >> +       { 0x1943, 0x0000, 0x00000103 },
> >> +       { 0x1944, 0x0000, 0x00000103 },
> >> +       { 0x1947, 0x0000, 0x00000103 },
> >> +};
> >
> > This is nearly impossible to handle.
> > Please add defines for 0x1, 0x103 and 0x303. Also please consider
> > adding comments for the devices.
> >
>
> Ack, Initially added the comments for devices, but since only driver is
> handling this data, and clients won't refer this so removed it. Will
> consider adding it back.

It will help developers / porters who will try matching the SID and the device.

>  This actlr field value might different (other
> than 0x1,0x103,0x303) in other platforms as per my anticipation,
> depending on the bit settings, so won't the defines change with
> different platforms? Hence this register setting value might be apt?

It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
provide sensible defines so that we can understand and review them.

Other platforms might use new defines.

>
> >> +
> >>   static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> >>   {
> >>          return container_of(smmu, struct qcom_smmu, smmu);
> >> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
> >>          .tlb_sync = qcom_smmu_tlb_sync,
> >>   };
> >>
> >> +
> >> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> >> +       .init_context = qcom_smmu_init_context,
> >> +       .cfg_probe = qcom_smmu_cfg_probe,
> >> +       .def_domain_type = qcom_smmu_def_domain_type,
> >> +       .reset = arm_mmu500_reset,
> >> +       .write_s2cr = qcom_smmu_write_s2cr,
> >> +       .tlb_sync = qcom_smmu_tlb_sync,
> >> +};
> >> +
> >>   static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> >>          .init_context = qcom_adreno_smmu_init_context,
> >>          .def_domain_type = qcom_smmu_def_domain_type,
> >> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
> >>          .reg_offset = qcom_smmu_impl0_reg_offset,
> >>   };
> >>
> >> +static const struct actlr_config sm8550_actlrcfg = {
> >> +       .adata = sm8550_apps_actlr_data,
> >> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> >> +};
> >> +
> >>   /*
> >>    * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
> >>    * there are not enough context banks.
> >> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
> >>          /* Also no debug configuration. */
> >>   };
> >>
> >> +
> >> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> >> +       .impl = &sm8550_smmu_500_impl,
> >> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
> >> +       .cfg = &qcom_smmu_impl0_cfg,
> >> +       .actlrcfg = &sm8550_actlrcfg,
> >> +};
> >
> > This structure doesn't seem to be linked. Did you test your patches?
> >
>
> Apologies Dmitry, just checked back my patches, I tested it but while
> refining the patches I somehow missed this link
> { .compatible = "qcom,sm8550-smmu-500", .data =
> &sm8550_smmu_500_impl0_data };
> in below qcom_smmu_500_impl0_data structure.
> I will take care of this in next version.
>
> >> +
> >>   static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
> >>          .impl = &qcom_smmu_500_impl,
> >>          .adreno_impl = &qcom_adreno_smmu_500_impl,
> >>          .cfg = &qcom_smmu_impl0_cfg,
> >>   };
> >>
> >> -/*
> >> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> >> - * special handling and can not be covered by the qcom,smmu-500 entry.
> >> - */
> >
> > Leave the comment in place.
> >
>
> Thanks for this comment which helped me to note the above mentioned
> mistake.
>
> >>   static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> >>          { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
> >>          { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> >> --
> >> 2.17.1
> >>
> >
> >
Konrad Dybcio Nov. 4, 2023, 11:29 a.m. UTC | #4
On 11/3/23 22:51, Bibek Kumar Patro wrote:
> Add ACTLR data table for SM8550 along with support for
> same including SM8550 specific implementation operations.
> 
> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
> ---
>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>   1 file changed, 81 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 68c1f4908473..590b7c285299 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -25,6 +25,64 @@ struct actlr_data {
>   	u32 actlr;
>   };
> 
> +static const struct actlr_data sm8550_apps_actlr_data[] = {
I assume this data will be different for each SoC.. perhaps
moving this to a separate file (not sure if dt makes sense if
it's hardcoded per platform) makes sense.

This will also assume that these can not differ between firmware
versions.

Konrad
kernel test robot Nov. 4, 2023, 3:29 p.m. UTC | #5
Hi Bibek,

kernel test robot noticed the following build warnings:

[auto build test WARNING on v6.6]
[also build test WARNING on linus/master next-20231103]
[cannot apply to joro-iommu/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Bibek-Kumar-Patro/iommu-arm-smmu-introduction-of-ACTLR-for-custom-prefetcher-settings/20231104-055625
base:   v6.6
patch link:    https://lore.kernel.org/r/20231103215124.1095-3-quic_bibekkum%40quicinc.com
patch subject: [PATCH 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231104/202311042304.pVLm1ZYk-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311042304.pVLm1ZYk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311042304.pVLm1ZYk-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:614:42: warning: 'sm8550_smmu_500_impl0_data' defined but not used [-Wunused-const-variable=]
     614 | static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
         |                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/sm8550_smmu_500_impl0_data +614 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c

   612	
   613	
 > 614	static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
   615		.impl = &sm8550_smmu_500_impl,
   616		.adreno_impl = &qcom_adreno_smmu_500_impl,
   617		.cfg = &qcom_smmu_impl0_cfg,
   618		.actlrcfg = &sm8550_actlrcfg,
   619	};
   620
Bibek Kumar Patro Nov. 6, 2023, 6:09 a.m. UTC | #6
On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
> On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
> <quic_bibekkum@quicinc.com> wrote:
>>
>>
>>
>> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
>>> On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
>>>>    1 file changed, 81 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> index 68c1f4908473..590b7c285299 100644
>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>> @@ -25,6 +25,64 @@ struct actlr_data {
>>>>           u32 actlr;
>>>>    };
>>>>
>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>>>> +       { 0x18a0, 0x0000, 0x00000103 },
>>>> +       { 0x18e0, 0x0000, 0x00000103 },
>>>> +       { 0x0800, 0x0020, 0x00000001 },
>>>> +       { 0x1800, 0x00c0, 0x00000001 },
>>>> +       { 0x1820, 0x0000, 0x00000001 },
>>>> +       { 0x1860, 0x0000, 0x00000001 },
>>>> +       { 0x0c01, 0x0020, 0x00000303 },
>>>> +       { 0x0c02, 0x0020, 0x00000303 },
>>>> +       { 0x0c03, 0x0020, 0x00000303 },
>>>> +       { 0x0c04, 0x0020, 0x00000303 },
>>>> +       { 0x0c05, 0x0020, 0x00000303 },
>>>> +       { 0x0c06, 0x0020, 0x00000303 },
>>>> +       { 0x0c07, 0x0020, 0x00000303 },
>>>> +       { 0x0c08, 0x0020, 0x00000303 },
>>>> +       { 0x0c09, 0x0020, 0x00000303 },
>>>> +       { 0x0c0c, 0x0020, 0x00000303 },
>>>> +       { 0x0c0d, 0x0020, 0x00000303 },
>>>> +       { 0x0c0e, 0x0020, 0x00000303 },
>>>> +       { 0x0c0f, 0x0020, 0x00000303 },
>>>> +       { 0x1961, 0x0000, 0x00000303 },
>>>> +       { 0x1962, 0x0000, 0x00000303 },
>>>> +       { 0x1963, 0x0000, 0x00000303 },
>>>> +       { 0x1964, 0x0000, 0x00000303 },
>>>> +       { 0x1965, 0x0000, 0x00000303 },
>>>> +       { 0x1966, 0x0000, 0x00000303 },
>>>> +       { 0x1967, 0x0000, 0x00000303 },
>>>> +       { 0x1968, 0x0000, 0x00000303 },
>>>> +       { 0x1969, 0x0000, 0x00000303 },
>>>> +       { 0x196c, 0x0000, 0x00000303 },
>>>> +       { 0x196d, 0x0000, 0x00000303 },
>>>> +       { 0x196e, 0x0000, 0x00000303 },
>>>> +       { 0x196f, 0x0000, 0x00000303 },
>>>> +       { 0x19c1, 0x0010, 0x00000303 },
>>>> +       { 0x19c2, 0x0010, 0x00000303 },
>>>> +       { 0x19c3, 0x0010, 0x00000303 },
>>>> +       { 0x19c4, 0x0010, 0x00000303 },
>>>> +       { 0x19c5, 0x0010, 0x00000303 },
>>>> +       { 0x19c6, 0x0010, 0x00000303 },
>>>> +       { 0x19c7, 0x0010, 0x00000303 },
>>>> +       { 0x19c8, 0x0010, 0x00000303 },
>>>> +       { 0x19c9, 0x0010, 0x00000303 },
>>>> +       { 0x19cc, 0x0010, 0x00000303 },
>>>> +       { 0x19cd, 0x0010, 0x00000303 },
>>>> +       { 0x19ce, 0x0010, 0x00000303 },
>>>> +       { 0x19cf, 0x0010, 0x00000303 },
>>>> +       { 0x1c00, 0x0002, 0x00000103 },
>>>> +       { 0x1c01, 0x0000, 0x00000001 },
>>>> +       { 0x1920, 0x0000, 0x00000103 },
>>>> +       { 0x1923, 0x0000, 0x00000103 },
>>>> +       { 0x1924, 0x0000, 0x00000103 },
>>>> +       { 0x1940, 0x0000, 0x00000103 },
>>>> +       { 0x1941, 0x0004, 0x00000103 },
>>>> +       { 0x1943, 0x0000, 0x00000103 },
>>>> +       { 0x1944, 0x0000, 0x00000103 },
>>>> +       { 0x1947, 0x0000, 0x00000103 },
>>>> +};
>>>
>>> This is nearly impossible to handle.
>>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
>>> adding comments for the devices.
>>>
>>
>> Ack, Initially added the comments for devices, but since only driver is
>> handling this data, and clients won't refer this so removed it. Will
>> consider adding it back.
> 
> It will help developers / porters who will try matching the SID and the device.
> 

Agree on the same, I'll add those comments for devices back.

>>   This actlr field value might different (other
>> than 0x1,0x103,0x303) in other platforms as per my anticipation,
>> depending on the bit settings, so won't the defines change with
>> different platforms? Hence this register setting value might be apt?
> 
> It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
> provide sensible defines so that we can understand and review them.
>

Understandable, In next patch I'll populate the actlr_data in
following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
the actlr values (0x1,0x103,0x303).
This would help in understanding these values. Hope this
proposed format will be okay?


> Other platforms might use new defines.
> 
>>
>>>> +
>>>>    static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>>>    {
>>>>           return container_of(smmu, struct qcom_smmu, smmu);
>>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>>>>           .tlb_sync = qcom_smmu_tlb_sync,
>>>>    };
>>>>
>>>> +
>>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>> +       .init_context = qcom_smmu_init_context,
>>>> +       .cfg_probe = qcom_smmu_cfg_probe,
>>>> +       .def_domain_type = qcom_smmu_def_domain_type,
>>>> +       .reset = arm_mmu500_reset,
>>>> +       .write_s2cr = qcom_smmu_write_s2cr,
>>>> +       .tlb_sync = qcom_smmu_tlb_sync,
>>>> +};
>>>> +
>>>>    static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>>           .init_context = qcom_adreno_smmu_init_context,
>>>>           .def_domain_type = qcom_smmu_def_domain_type,
>>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>>>>           .reg_offset = qcom_smmu_impl0_reg_offset,
>>>>    };
>>>>
>>>> +static const struct actlr_config sm8550_actlrcfg = {
>>>> +       .adata = sm8550_apps_actlr_data,
>>>> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>>>> +};
>>>> +
>>>>    /*
>>>>     * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>>>>     * there are not enough context banks.
>>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>>>>           /* Also no debug configuration. */
>>>>    };
>>>>
>>>> +
>>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>>>> +       .impl = &sm8550_smmu_500_impl,
>>>> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>> +       .cfg = &qcom_smmu_impl0_cfg,
>>>> +       .actlrcfg = &sm8550_actlrcfg,
>>>> +};
>>>
>>> This structure doesn't seem to be linked. Did you test your patches?
>>>
>>
>> Apologies Dmitry, just checked back my patches, I tested it but while
>> refining the patches I somehow missed this link
>> { .compatible = "qcom,sm8550-smmu-500", .data =
>> &sm8550_smmu_500_impl0_data };
>> in below qcom_smmu_500_impl0_data structure.
>> I will take care of this in next version.
>>
>>>> +
>>>>    static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>>>>           .impl = &qcom_smmu_500_impl,
>>>>           .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>           .cfg = &qcom_smmu_impl0_cfg,
>>>>    };
>>>>
>>>> -/*
>>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>>>> - */
>>>
>>> Leave the comment in place.
>>>
>>
>> Thanks for this comment which helped me to note the above mentioned
>> mistake.
>>
>>>>    static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>>>>           { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>>>>           { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>>>> --
>>>> 2.17.1
>>>>
>>>
>>>
> 
> 
>
Bibek Kumar Patro Nov. 6, 2023, 6:26 a.m. UTC | #7
On 11/4/2023 4:59 PM, Konrad Dybcio wrote:
> 
> 
> On 11/3/23 22:51, Bibek Kumar Patro wrote:
>> Add ACTLR data table for SM8550 along with support for
>> same including SM8550 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
>> ---
>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 85 +++++++++++++++++++++-
>>   1 file changed, 81 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 68c1f4908473..590b7c285299 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -25,6 +25,64 @@ struct actlr_data {
>>       u32 actlr;
>>   };
>>
>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> I assume this data will be different for each SoC.. perhaps
> moving this to a separate file (not sure if dt makes sense if
> it's hardcoded per platform) makes sense.
> 

Yes, this data will be different for each SoC.
Right, adding these properties in dt won't be a right thing
since this is a register setting and not a hardware defining property.
As per my understanding passing register content via device tree is 
highly discouraged, so hosting this data inside the driver.
For reference, adding the RFC link archiving this discussion:
https://lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/#t
If my recollection is correct, some drivers like llcc is also
following similar implementation
drivers/phy/qualcomm/phy-qcom-qmp-combo.c
drivers/soc/qcom/llcc-qcom.c


> This will also assume that these can not differ between firmware
> versions.
>

Right, these won't differ between firmware versions.

Thanks & regards,
Bibek

> Konrad
Dmitry Baryshkov Nov. 6, 2023, 9:05 a.m. UTC | #8
On Mon, 6 Nov 2023 at 08:10, Bibek Kumar Patro
<quic_bibekkum@quicinc.com> wrote:
>
>
>
> On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
> > On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
> > <quic_bibekkum@quicinc.com> wrote:
> >>
> >>
> >>
> >> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
> >>> On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
> >>>>    1 file changed, 81 insertions(+), 4 deletions(-)
> >>>>
> >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> index 68c1f4908473..590b7c285299 100644
> >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> >>>> @@ -25,6 +25,64 @@ struct actlr_data {
> >>>>           u32 actlr;
> >>>>    };
> >>>>
> >>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
> >>>> +       { 0x18a0, 0x0000, 0x00000103 },
> >>>> +       { 0x18e0, 0x0000, 0x00000103 },
> >>>> +       { 0x0800, 0x0020, 0x00000001 },
> >>>> +       { 0x1800, 0x00c0, 0x00000001 },
> >>>> +       { 0x1820, 0x0000, 0x00000001 },
> >>>> +       { 0x1860, 0x0000, 0x00000001 },
> >>>> +       { 0x0c01, 0x0020, 0x00000303 },
> >>>> +       { 0x0c02, 0x0020, 0x00000303 },
> >>>> +       { 0x0c03, 0x0020, 0x00000303 },
> >>>> +       { 0x0c04, 0x0020, 0x00000303 },
> >>>> +       { 0x0c05, 0x0020, 0x00000303 },
> >>>> +       { 0x0c06, 0x0020, 0x00000303 },
> >>>> +       { 0x0c07, 0x0020, 0x00000303 },
> >>>> +       { 0x0c08, 0x0020, 0x00000303 },
> >>>> +       { 0x0c09, 0x0020, 0x00000303 },
> >>>> +       { 0x0c0c, 0x0020, 0x00000303 },
> >>>> +       { 0x0c0d, 0x0020, 0x00000303 },
> >>>> +       { 0x0c0e, 0x0020, 0x00000303 },
> >>>> +       { 0x0c0f, 0x0020, 0x00000303 },
> >>>> +       { 0x1961, 0x0000, 0x00000303 },
> >>>> +       { 0x1962, 0x0000, 0x00000303 },
> >>>> +       { 0x1963, 0x0000, 0x00000303 },
> >>>> +       { 0x1964, 0x0000, 0x00000303 },
> >>>> +       { 0x1965, 0x0000, 0x00000303 },
> >>>> +       { 0x1966, 0x0000, 0x00000303 },
> >>>> +       { 0x1967, 0x0000, 0x00000303 },
> >>>> +       { 0x1968, 0x0000, 0x00000303 },
> >>>> +       { 0x1969, 0x0000, 0x00000303 },
> >>>> +       { 0x196c, 0x0000, 0x00000303 },
> >>>> +       { 0x196d, 0x0000, 0x00000303 },
> >>>> +       { 0x196e, 0x0000, 0x00000303 },
> >>>> +       { 0x196f, 0x0000, 0x00000303 },
> >>>> +       { 0x19c1, 0x0010, 0x00000303 },
> >>>> +       { 0x19c2, 0x0010, 0x00000303 },
> >>>> +       { 0x19c3, 0x0010, 0x00000303 },
> >>>> +       { 0x19c4, 0x0010, 0x00000303 },
> >>>> +       { 0x19c5, 0x0010, 0x00000303 },
> >>>> +       { 0x19c6, 0x0010, 0x00000303 },
> >>>> +       { 0x19c7, 0x0010, 0x00000303 },
> >>>> +       { 0x19c8, 0x0010, 0x00000303 },
> >>>> +       { 0x19c9, 0x0010, 0x00000303 },
> >>>> +       { 0x19cc, 0x0010, 0x00000303 },
> >>>> +       { 0x19cd, 0x0010, 0x00000303 },
> >>>> +       { 0x19ce, 0x0010, 0x00000303 },
> >>>> +       { 0x19cf, 0x0010, 0x00000303 },
> >>>> +       { 0x1c00, 0x0002, 0x00000103 },
> >>>> +       { 0x1c01, 0x0000, 0x00000001 },
> >>>> +       { 0x1920, 0x0000, 0x00000103 },
> >>>> +       { 0x1923, 0x0000, 0x00000103 },
> >>>> +       { 0x1924, 0x0000, 0x00000103 },
> >>>> +       { 0x1940, 0x0000, 0x00000103 },
> >>>> +       { 0x1941, 0x0004, 0x00000103 },
> >>>> +       { 0x1943, 0x0000, 0x00000103 },
> >>>> +       { 0x1944, 0x0000, 0x00000103 },
> >>>> +       { 0x1947, 0x0000, 0x00000103 },
> >>>> +};
> >>>
> >>> This is nearly impossible to handle.
> >>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
> >>> adding comments for the devices.
> >>>
> >>
> >> Ack, Initially added the comments for devices, but since only driver is
> >> handling this data, and clients won't refer this so removed it. Will
> >> consider adding it back.
> >
> > It will help developers / porters who will try matching the SID and the device.
> >
>
> Agree on the same, I'll add those comments for devices back.
>
> >>   This actlr field value might different (other
> >> than 0x1,0x103,0x303) in other platforms as per my anticipation,
> >> depending on the bit settings, so won't the defines change with
> >> different platforms? Hence this register setting value might be apt?
> >
> > It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
> > provide sensible defines so that we can understand and review them.
> >
>
> Understandable, In next patch I'll populate the actlr_data in
> following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
> where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
> the actlr values (0x1,0x103,0x303).
> This would help in understanding these values. Hope this
> proposed format will be okay?

Yes, this sounds good. Please also add description for CPRE and CMTLB bits.

>
>
> > Other platforms might use new defines.
> >
> >>
> >>>> +
> >>>>    static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
> >>>>    {
> >>>>           return container_of(smmu, struct qcom_smmu, smmu);
> >>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
> >>>>           .tlb_sync = qcom_smmu_tlb_sync,
> >>>>    };
> >>>>
> >>>> +
> >>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
> >>>> +       .init_context = qcom_smmu_init_context,
> >>>> +       .cfg_probe = qcom_smmu_cfg_probe,
> >>>> +       .def_domain_type = qcom_smmu_def_domain_type,
> >>>> +       .reset = arm_mmu500_reset,
> >>>> +       .write_s2cr = qcom_smmu_write_s2cr,
> >>>> +       .tlb_sync = qcom_smmu_tlb_sync,
> >>>> +};
> >>>> +
> >>>>    static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
> >>>>           .init_context = qcom_adreno_smmu_init_context,
> >>>>           .def_domain_type = qcom_smmu_def_domain_type,
> >>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
> >>>>           .reg_offset = qcom_smmu_impl0_reg_offset,
> >>>>    };
> >>>>
> >>>> +static const struct actlr_config sm8550_actlrcfg = {
> >>>> +       .adata = sm8550_apps_actlr_data,
> >>>> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
> >>>> +};
> >>>> +
> >>>>    /*
> >>>>     * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
> >>>>     * there are not enough context banks.
> >>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
> >>>>           /* Also no debug configuration. */
> >>>>    };
> >>>>
> >>>> +
> >>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
> >>>> +       .impl = &sm8550_smmu_500_impl,
> >>>> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
> >>>> +       .cfg = &qcom_smmu_impl0_cfg,
> >>>> +       .actlrcfg = &sm8550_actlrcfg,
> >>>> +};
> >>>
> >>> This structure doesn't seem to be linked. Did you test your patches?
> >>>
> >>
> >> Apologies Dmitry, just checked back my patches, I tested it but while
> >> refining the patches I somehow missed this link
> >> { .compatible = "qcom,sm8550-smmu-500", .data =
> >> &sm8550_smmu_500_impl0_data };
> >> in below qcom_smmu_500_impl0_data structure.
> >> I will take care of this in next version.
> >>
> >>>> +
> >>>>    static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
> >>>>           .impl = &qcom_smmu_500_impl,
> >>>>           .adreno_impl = &qcom_adreno_smmu_500_impl,
> >>>>           .cfg = &qcom_smmu_impl0_cfg,
> >>>>    };
> >>>>
> >>>> -/*
> >>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
> >>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
> >>>> - */
> >>>
> >>> Leave the comment in place.
> >>>
> >>
> >> Thanks for this comment which helped me to note the above mentioned
> >> mistake.
> >>
> >>>>    static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
> >>>>           { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
> >>>>           { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
> >>>> --
> >>>> 2.17.1
> >>>>
> >>>
> >>>
> >
> >
> >
Bibek Kumar Patro Nov. 6, 2023, 12:09 p.m. UTC | #9
On 11/6/2023 2:35 PM, Dmitry Baryshkov wrote:
> On Mon, 6 Nov 2023 at 08:10, Bibek Kumar Patro
> <quic_bibekkum@quicinc.com> wrote:
>>
>>
>>
>> On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
>>> On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
>>> <quic_bibekkum@quicinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, 3 Nov 2023 at 23:53, 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 | 85 +++++++++++++++++++++-
>>>>>>     1 file changed, 81 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> index 68c1f4908473..590b7c285299 100644
>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>> @@ -25,6 +25,64 @@ struct actlr_data {
>>>>>>            u32 actlr;
>>>>>>     };
>>>>>>
>>>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>>>>>> +       { 0x18a0, 0x0000, 0x00000103 },
>>>>>> +       { 0x18e0, 0x0000, 0x00000103 },
>>>>>> +       { 0x0800, 0x0020, 0x00000001 },
>>>>>> +       { 0x1800, 0x00c0, 0x00000001 },
>>>>>> +       { 0x1820, 0x0000, 0x00000001 },
>>>>>> +       { 0x1860, 0x0000, 0x00000001 },
>>>>>> +       { 0x0c01, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c02, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c03, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c04, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c05, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c06, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c07, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c08, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c09, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c0c, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c0d, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c0e, 0x0020, 0x00000303 },
>>>>>> +       { 0x0c0f, 0x0020, 0x00000303 },
>>>>>> +       { 0x1961, 0x0000, 0x00000303 },
>>>>>> +       { 0x1962, 0x0000, 0x00000303 },
>>>>>> +       { 0x1963, 0x0000, 0x00000303 },
>>>>>> +       { 0x1964, 0x0000, 0x00000303 },
>>>>>> +       { 0x1965, 0x0000, 0x00000303 },
>>>>>> +       { 0x1966, 0x0000, 0x00000303 },
>>>>>> +       { 0x1967, 0x0000, 0x00000303 },
>>>>>> +       { 0x1968, 0x0000, 0x00000303 },
>>>>>> +       { 0x1969, 0x0000, 0x00000303 },
>>>>>> +       { 0x196c, 0x0000, 0x00000303 },
>>>>>> +       { 0x196d, 0x0000, 0x00000303 },
>>>>>> +       { 0x196e, 0x0000, 0x00000303 },
>>>>>> +       { 0x196f, 0x0000, 0x00000303 },
>>>>>> +       { 0x19c1, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c2, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c3, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c4, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c5, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c6, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c7, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c8, 0x0010, 0x00000303 },
>>>>>> +       { 0x19c9, 0x0010, 0x00000303 },
>>>>>> +       { 0x19cc, 0x0010, 0x00000303 },
>>>>>> +       { 0x19cd, 0x0010, 0x00000303 },
>>>>>> +       { 0x19ce, 0x0010, 0x00000303 },
>>>>>> +       { 0x19cf, 0x0010, 0x00000303 },
>>>>>> +       { 0x1c00, 0x0002, 0x00000103 },
>>>>>> +       { 0x1c01, 0x0000, 0x00000001 },
>>>>>> +       { 0x1920, 0x0000, 0x00000103 },
>>>>>> +       { 0x1923, 0x0000, 0x00000103 },
>>>>>> +       { 0x1924, 0x0000, 0x00000103 },
>>>>>> +       { 0x1940, 0x0000, 0x00000103 },
>>>>>> +       { 0x1941, 0x0004, 0x00000103 },
>>>>>> +       { 0x1943, 0x0000, 0x00000103 },
>>>>>> +       { 0x1944, 0x0000, 0x00000103 },
>>>>>> +       { 0x1947, 0x0000, 0x00000103 },
>>>>>> +};
>>>>>
>>>>> This is nearly impossible to handle.
>>>>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
>>>>> adding comments for the devices.
>>>>>
>>>>
>>>> Ack, Initially added the comments for devices, but since only driver is
>>>> handling this data, and clients won't refer this so removed it. Will
>>>> consider adding it back.
>>>
>>> It will help developers / porters who will try matching the SID and the device.
>>>
>>
>> Agree on the same, I'll add those comments for devices back.
>>
>>>>    This actlr field value might different (other
>>>> than 0x1,0x103,0x303) in other platforms as per my anticipation,
>>>> depending on the bit settings, so won't the defines change with
>>>> different platforms? Hence this register setting value might be apt?
>>>
>>> It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
>>> provide sensible defines so that we can understand and review them.
>>>
>>
>> Understandable, In next patch I'll populate the actlr_data in
>> following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
>> where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
>> the actlr values (0x1,0x103,0x303).
>> This would help in understanding these values. Hope this
>> proposed format will be okay?
> 
> Yes, this sounds good. Please also add description for CPRE and CMTLB bits.
> 

Thanks for the confirmation. Sure will add the descriptions for
CPRE and CMTLB bits as well in v2

>>
>>
>>> Other platforms might use new defines.
>>>
>>>>
>>>>>> +
>>>>>>     static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>>>>>     {
>>>>>>            return container_of(smmu, struct qcom_smmu, smmu);
>>>>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl sdm845_smmu_500_impl = {
>>>>>>            .tlb_sync = qcom_smmu_tlb_sync,
>>>>>>     };
>>>>>>
>>>>>> +
>>>>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>>>> +       .init_context = qcom_smmu_init_context,
>>>>>> +       .cfg_probe = qcom_smmu_cfg_probe,
>>>>>> +       .def_domain_type = qcom_smmu_def_domain_type,
>>>>>> +       .reset = arm_mmu500_reset,
>>>>>> +       .write_s2cr = qcom_smmu_write_s2cr,
>>>>>> +       .tlb_sync = qcom_smmu_tlb_sync,
>>>>>> +};
>>>>>> +
>>>>>>     static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>>>>            .init_context = qcom_adreno_smmu_init_context,
>>>>>>            .def_domain_type = qcom_smmu_def_domain_type,
>>>>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
>>>>>>            .reg_offset = qcom_smmu_impl0_reg_offset,
>>>>>>     };
>>>>>>
>>>>>> +static const struct actlr_config sm8550_actlrcfg = {
>>>>>> +       .adata = sm8550_apps_actlr_data,
>>>>>> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>>>>>> +};
>>>>>> +
>>>>>>     /*
>>>>>>      * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
>>>>>>      * there are not enough context banks.
>>>>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
>>>>>>            /* Also no debug configuration. */
>>>>>>     };
>>>>>>
>>>>>> +
>>>>>> +static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
>>>>>> +       .impl = &sm8550_smmu_500_impl,
>>>>>> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>> +       .cfg = &qcom_smmu_impl0_cfg,
>>>>>> +       .actlrcfg = &sm8550_actlrcfg,
>>>>>> +};
>>>>>
>>>>> This structure doesn't seem to be linked. Did you test your patches?
>>>>>
>>>>
>>>> Apologies Dmitry, just checked back my patches, I tested it but while
>>>> refining the patches I somehow missed this link
>>>> { .compatible = "qcom,sm8550-smmu-500", .data =
>>>> &sm8550_smmu_500_impl0_data };
>>>> in below qcom_smmu_500_impl0_data structure.
>>>> I will take care of this in next version.
>>>>
>>>>>> +
>>>>>>     static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
>>>>>>            .impl = &qcom_smmu_500_impl,
>>>>>>            .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>>            .cfg = &qcom_smmu_impl0_cfg,
>>>>>>     };
>>>>>>
>>>>>> -/*
>>>>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
>>>>>> - * special handling and can not be covered by the qcom,smmu-500 entry.
>>>>>> - */
>>>>>
>>>>> Leave the comment in place.
>>>>>
>>>>
>>>> Thanks for this comment which helped me to note the above mentioned
>>>> mistake.
>>>>
>>>>>>     static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
>>>>>>            { .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
>>>>>>            { .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
> 
> 
>
Pratyush Brahma Nov. 10, 2023, 9:55 a.m. UTC | #10
On 06-11-2023 17:39, Bibek Kumar Patro wrote:
>
>
> On 11/6/2023 2:35 PM, Dmitry Baryshkov wrote:
>> On Mon, 6 Nov 2023 at 08:10, Bibek Kumar Patro
>> <quic_bibekkum@quicinc.com> wrote:
>>>
>>>
>>>
>>> On 11/4/2023 4:11 AM, Dmitry Baryshkov wrote:
>>>> On Sat, 4 Nov 2023 at 00:38, Bibek Kumar Patro
>>>> <quic_bibekkum@quicinc.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 11/4/2023 3:31 AM, Dmitry Baryshkov wrote:
>>>>>> On Fri, 3 Nov 2023 at 23:53, 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 | 85 
>>>>>>> +++++++++++++++++++++-
>>>>>>>     1 file changed, 81 insertions(+), 4 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
>>>>>>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>> index 68c1f4908473..590b7c285299 100644
>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>> @@ -25,6 +25,64 @@ struct actlr_data {
>>>>>>>            u32 actlr;
>>>>>>>     };
>>>>>>>
>>>>>>> +static const struct actlr_data sm8550_apps_actlr_data[] = {
>>>>>>> +       { 0x18a0, 0x0000, 0x00000103 },
>>>>>>> +       { 0x18e0, 0x0000, 0x00000103 },
>>>>>>> +       { 0x0800, 0x0020, 0x00000001 },
>>>>>>> +       { 0x1800, 0x00c0, 0x00000001 },
>>>>>>> +       { 0x1820, 0x0000, 0x00000001 },
>>>>>>> +       { 0x1860, 0x0000, 0x00000001 },
>>>>>>> +       { 0x0c01, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c02, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c03, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c04, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c05, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c06, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c07, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c08, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c09, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c0c, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c0d, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c0e, 0x0020, 0x00000303 },
>>>>>>> +       { 0x0c0f, 0x0020, 0x00000303 },
>>>>>>> +       { 0x1961, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1962, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1963, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1964, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1965, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1966, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1967, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1968, 0x0000, 0x00000303 },
>>>>>>> +       { 0x1969, 0x0000, 0x00000303 },
>>>>>>> +       { 0x196c, 0x0000, 0x00000303 },
>>>>>>> +       { 0x196d, 0x0000, 0x00000303 },
>>>>>>> +       { 0x196e, 0x0000, 0x00000303 },
>>>>>>> +       { 0x196f, 0x0000, 0x00000303 },
>>>>>>> +       { 0x19c1, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c2, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c3, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c4, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c5, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c6, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c7, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c8, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19c9, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19cc, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19cd, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19ce, 0x0010, 0x00000303 },
>>>>>>> +       { 0x19cf, 0x0010, 0x00000303 },
>>>>>>> +       { 0x1c00, 0x0002, 0x00000103 },
>>>>>>> +       { 0x1c01, 0x0000, 0x00000001 },
>>>>>>> +       { 0x1920, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1923, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1924, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1940, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1941, 0x0004, 0x00000103 },
>>>>>>> +       { 0x1943, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1944, 0x0000, 0x00000103 },
>>>>>>> +       { 0x1947, 0x0000, 0x00000103 },
>>>>>>> +};
>>>>>>
>>>>>> This is nearly impossible to handle.
>>>>>> Please add defines for 0x1, 0x103 and 0x303. Also please consider
>>>>>> adding comments for the devices.
>>>>>>
>>>>>
>>>>> Ack, Initially added the comments for devices, but since only 
>>>>> driver is
>>>>> handling this data, and clients won't refer this so removed it. Will
>>>>> consider adding it back.
>>>>
>>>> It will help developers / porters who will try matching the SID and 
>>>> the device.
>>>>
>>>
>>> Agree on the same, I'll add those comments for devices back.
TBH, I don't see much value add in adding comments for device names as 
developers
are aware of the SIDs they use and can refer to them in the table if 
they wish to.
Secondly, developers need not care about this configuration unless they 
want to modify it which
  is not the case here as these are platform specific configurations. I 
think, only adding
the defines for the actlr reg values should suffice.
>>>>>    This actlr field value might different (other
>>>>> than 0x1,0x103,0x303) in other platforms as per my anticipation,
>>>>> depending on the bit settings, so won't the defines change with
>>>>> different platforms? Hence this register setting value might be apt?
>>>>
>>>> It is simple. 0x1, 0x103 and 0x303 are pure magic values. Please
>>>> provide sensible defines so that we can understand and review them.
>>>>
>>>
>>> Understandable, In next patch I'll populate the actlr_data in
>>> following format { SID, MASK, PRE_FETCH_n | CPRE | CMTLB }.
>>> where " PRE_FETCH_n | CPRE | CMTLB " will be defines for
>>> the actlr values (0x1,0x103,0x303).
>>> This would help in understanding these values. Hope this
>>> proposed format will be okay?
>>
>> Yes, this sounds good. Please also add description for CPRE and CMTLB 
>> bits.
>>
>
> Thanks for the confirmation. Sure will add the descriptions for
> CPRE and CMTLB bits as well in v2
>
>>>
>>>
>>>> Other platforms might use new defines.
>>>>
>>>>>
>>>>>>> +
>>>>>>>     static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device 
>>>>>>> *smmu)
>>>>>>>     {
>>>>>>>            return container_of(smmu, struct qcom_smmu, smmu);
>>>>>>> @@ -444,6 +502,16 @@ static const struct arm_smmu_impl 
>>>>>>> sdm845_smmu_500_impl = {
>>>>>>>            .tlb_sync = qcom_smmu_tlb_sync,
>>>>>>>     };
>>>>>>>
>>>>>>> +
>>>>>>> +static const struct arm_smmu_impl sm8550_smmu_500_impl = {
>>>>>>> +       .init_context = qcom_smmu_init_context,
>>>>>>> +       .cfg_probe = qcom_smmu_cfg_probe,
>>>>>>> +       .def_domain_type = qcom_smmu_def_domain_type,
>>>>>>> +       .reset = arm_mmu500_reset,
>>>>>>> +       .write_s2cr = qcom_smmu_write_s2cr,
>>>>>>> +       .tlb_sync = qcom_smmu_tlb_sync,
>>>>>>> +};
>>>>>>> +
>>>>>>>     static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
>>>>>>>            .init_context = qcom_adreno_smmu_init_context,
>>>>>>>            .def_domain_type = qcom_smmu_def_domain_type,
>>>>>>> @@ -507,6 +575,11 @@ static const struct qcom_smmu_config 
>>>>>>> qcom_smmu_impl0_cfg = {
>>>>>>>            .reg_offset = qcom_smmu_impl0_reg_offset,
>>>>>>>     };
>>>>>>>
>>>>>>> +static const struct actlr_config sm8550_actlrcfg = {
>>>>>>> +       .adata = sm8550_apps_actlr_data,
>>>>>>> +       .size = ARRAY_SIZE(sm8550_apps_actlr_data),
>>>>>>> +};
>>>>>>> +
>>>>>>>     /*
>>>>>>>      * It is not yet possible to use MDP SMMU with the bypass 
>>>>>>> quirk on the msm8996,
>>>>>>>      * there are not enough context banks.
>>>>>>> @@ -530,16 +603,20 @@ static const struct qcom_smmu_match_data 
>>>>>>> sdm845_smmu_500_data = {
>>>>>>>            /* Also no debug configuration. */
>>>>>>>     };
>>>>>>>
>>>>>>> +
>>>>>>> +static const struct qcom_smmu_match_data 
>>>>>>> sm8550_smmu_500_impl0_data = {
>>>>>>> +       .impl = &sm8550_smmu_500_impl,
>>>>>>> +       .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>>> +       .cfg = &qcom_smmu_impl0_cfg,
>>>>>>> +       .actlrcfg = &sm8550_actlrcfg,
>>>>>>> +};
>>>>>>
>>>>>> This structure doesn't seem to be linked. Did you test your patches?
>>>>>>
>>>>>
>>>>> Apologies Dmitry, just checked back my patches, I tested it but while
>>>>> refining the patches I somehow missed this link
>>>>> { .compatible = "qcom,sm8550-smmu-500", .data =
>>>>> &sm8550_smmu_500_impl0_data };
>>>>> in below qcom_smmu_500_impl0_data structure.
>>>>> I will take care of this in next version.
>>>>>
>>>>>>> +
>>>>>>>     static const struct qcom_smmu_match_data 
>>>>>>> qcom_smmu_500_impl0_data = {
>>>>>>>            .impl = &qcom_smmu_500_impl,
>>>>>>>            .adreno_impl = &qcom_adreno_smmu_500_impl,
>>>>>>>            .cfg = &qcom_smmu_impl0_cfg,
>>>>>>>     };
>>>>>>>
>>>>>>> -/*
>>>>>>> - * Do not add any more qcom,SOC-smmu-500 entries to this list, 
>>>>>>> unless they need
>>>>>>> - * special handling and can not be covered by the qcom,smmu-500 
>>>>>>> entry.
>>>>>>> - */
>>>>>>
>>>>>> Leave the comment in place.
>>>>>>
>>>>>
>>>>> Thanks for this comment which helped me to note the above mentioned
>>>>> mistake.
>>>>>
>>>>>>>     static const struct of_device_id __maybe_unused 
>>>>>>> qcom_smmu_impl_of_match[] = {
>>>>>>>            { .compatible = "qcom,msm8996-smmu-v2", .data = 
>>>>>>> &msm8996_smmu_data },
>>>>>>>            { .compatible = "qcom,msm8998-smmu-v2", .data = 
>>>>>>> &qcom_smmu_v2_data },
>>>>>>> -- 
>>>>>>> 2.17.1
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>>>
>>
>>
>>
Thanks,

Pratyush
diff mbox series

Patch

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 68c1f4908473..590b7c285299 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -25,6 +25,64 @@  struct actlr_data {
 	u32 actlr;
 };

+static const struct actlr_data sm8550_apps_actlr_data[] = {
+	{ 0x18a0, 0x0000, 0x00000103 },
+	{ 0x18e0, 0x0000, 0x00000103 },
+	{ 0x0800, 0x0020, 0x00000001 },
+	{ 0x1800, 0x00c0, 0x00000001 },
+	{ 0x1820, 0x0000, 0x00000001 },
+	{ 0x1860, 0x0000, 0x00000001 },
+	{ 0x0c01, 0x0020, 0x00000303 },
+	{ 0x0c02, 0x0020, 0x00000303 },
+	{ 0x0c03, 0x0020, 0x00000303 },
+	{ 0x0c04, 0x0020, 0x00000303 },
+	{ 0x0c05, 0x0020, 0x00000303 },
+	{ 0x0c06, 0x0020, 0x00000303 },
+	{ 0x0c07, 0x0020, 0x00000303 },
+	{ 0x0c08, 0x0020, 0x00000303 },
+	{ 0x0c09, 0x0020, 0x00000303 },
+	{ 0x0c0c, 0x0020, 0x00000303 },
+	{ 0x0c0d, 0x0020, 0x00000303 },
+	{ 0x0c0e, 0x0020, 0x00000303 },
+	{ 0x0c0f, 0x0020, 0x00000303 },
+	{ 0x1961, 0x0000, 0x00000303 },
+	{ 0x1962, 0x0000, 0x00000303 },
+	{ 0x1963, 0x0000, 0x00000303 },
+	{ 0x1964, 0x0000, 0x00000303 },
+	{ 0x1965, 0x0000, 0x00000303 },
+	{ 0x1966, 0x0000, 0x00000303 },
+	{ 0x1967, 0x0000, 0x00000303 },
+	{ 0x1968, 0x0000, 0x00000303 },
+	{ 0x1969, 0x0000, 0x00000303 },
+	{ 0x196c, 0x0000, 0x00000303 },
+	{ 0x196d, 0x0000, 0x00000303 },
+	{ 0x196e, 0x0000, 0x00000303 },
+	{ 0x196f, 0x0000, 0x00000303 },
+	{ 0x19c1, 0x0010, 0x00000303 },
+	{ 0x19c2, 0x0010, 0x00000303 },
+	{ 0x19c3, 0x0010, 0x00000303 },
+	{ 0x19c4, 0x0010, 0x00000303 },
+	{ 0x19c5, 0x0010, 0x00000303 },
+	{ 0x19c6, 0x0010, 0x00000303 },
+	{ 0x19c7, 0x0010, 0x00000303 },
+	{ 0x19c8, 0x0010, 0x00000303 },
+	{ 0x19c9, 0x0010, 0x00000303 },
+	{ 0x19cc, 0x0010, 0x00000303 },
+	{ 0x19cd, 0x0010, 0x00000303 },
+	{ 0x19ce, 0x0010, 0x00000303 },
+	{ 0x19cf, 0x0010, 0x00000303 },
+	{ 0x1c00, 0x0002, 0x00000103 },
+	{ 0x1c01, 0x0000, 0x00000001 },
+	{ 0x1920, 0x0000, 0x00000103 },
+	{ 0x1923, 0x0000, 0x00000103 },
+	{ 0x1924, 0x0000, 0x00000103 },
+	{ 0x1940, 0x0000, 0x00000103 },
+	{ 0x1941, 0x0004, 0x00000103 },
+	{ 0x1943, 0x0000, 0x00000103 },
+	{ 0x1944, 0x0000, 0x00000103 },
+	{ 0x1947, 0x0000, 0x00000103 },
+};
+
 static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
 {
 	return container_of(smmu, struct qcom_smmu, smmu);
@@ -444,6 +502,16 @@  static const struct arm_smmu_impl sdm845_smmu_500_impl = {
 	.tlb_sync = qcom_smmu_tlb_sync,
 };

+
+static const struct arm_smmu_impl sm8550_smmu_500_impl = {
+	.init_context = qcom_smmu_init_context,
+	.cfg_probe = qcom_smmu_cfg_probe,
+	.def_domain_type = qcom_smmu_def_domain_type,
+	.reset = arm_mmu500_reset,
+	.write_s2cr = qcom_smmu_write_s2cr,
+	.tlb_sync = qcom_smmu_tlb_sync,
+};
+
 static const struct arm_smmu_impl qcom_adreno_smmu_v2_impl = {
 	.init_context = qcom_adreno_smmu_init_context,
 	.def_domain_type = qcom_smmu_def_domain_type,
@@ -507,6 +575,11 @@  static const struct qcom_smmu_config qcom_smmu_impl0_cfg = {
 	.reg_offset = qcom_smmu_impl0_reg_offset,
 };

+static const struct actlr_config sm8550_actlrcfg = {
+	.adata = sm8550_apps_actlr_data,
+	.size = ARRAY_SIZE(sm8550_apps_actlr_data),
+};
+
 /*
  * It is not yet possible to use MDP SMMU with the bypass quirk on the msm8996,
  * there are not enough context banks.
@@ -530,16 +603,20 @@  static const struct qcom_smmu_match_data sdm845_smmu_500_data = {
 	/* Also no debug configuration. */
 };

+
+static const struct qcom_smmu_match_data sm8550_smmu_500_impl0_data = {
+	.impl = &sm8550_smmu_500_impl,
+	.adreno_impl = &qcom_adreno_smmu_500_impl,
+	.cfg = &qcom_smmu_impl0_cfg,
+	.actlrcfg = &sm8550_actlrcfg,
+};
+
 static const struct qcom_smmu_match_data qcom_smmu_500_impl0_data = {
 	.impl = &qcom_smmu_500_impl,
 	.adreno_impl = &qcom_adreno_smmu_500_impl,
 	.cfg = &qcom_smmu_impl0_cfg,
 };

-/*
- * Do not add any more qcom,SOC-smmu-500 entries to this list, unless they need
- * special handling and can not be covered by the qcom,smmu-500 entry.
- */
 static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
 	{ .compatible = "qcom,msm8996-smmu-v2", .data = &msm8996_smmu_data },
 	{ .compatible = "qcom,msm8998-smmu-v2", .data = &qcom_smmu_v2_data },