diff mbox series

[v14,1/6] iommu/arm-smmu: re-enable context caching in smmu reset operation

Message ID 20240816174259.2056829-2-quic_bibekkum@quicinc.com (mailing list archive)
State Superseded
Headers show
Series iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs | expand

Commit Message

Bibek Kumar Patro Aug. 16, 2024, 5:42 p.m. UTC 
Default MMU-500 reset operation disables context caching in
prefetch buffer. It is however expected for context banks using
the ACTLR register to retain their prefetch value during reset
and runtime suspend.

Replace default MMU-500 reset operation with Qualcomm specific reset
operation which envelope the default reset operation and re-enables
context caching in prefetch buffer for Qualcomm SoCs.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 36 ++++++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

--
2.34.1

Comments

Robin Murphy Aug. 30, 2024, 12:53 p.m. UTC | #1 
On 16/08/2024 6:42 pm, Bibek Kumar Patro wrote:
> Default MMU-500 reset operation disables context caching in
> prefetch buffer. It is however expected for context banks using
> the ACTLR register to retain their prefetch value during reset
> and runtime suspend.
> 
> Replace default MMU-500 reset operation with Qualcomm specific reset
> operation which envelope the default reset operation and re-enables
> context caching in prefetch buffer for Qualcomm SoCs.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
> ---
>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 36 ++++++++++++++++++++--
>   1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 36c6b36ad4ff..8ac1850b852f 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -16,6 +16,16 @@
> 
>   #define QCOM_DUMMY_VAL	-1
> 
> +/*
> + * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the
> + * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch
> + * buffer). The remaining bits are implementation defined and vary across
> + * SoCs.
> + */
> +
> +#define CPRE			(1 << 1)
> +#define CMTLB			(1 << 0)
> +
>   static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>   {
>   	return container_of(smmu, struct qcom_smmu, smmu);
> @@ -381,11 +391,31 @@ static int qcom_smmu_def_domain_type(struct device *dev)
>   	return match ? IOMMU_DOMAIN_IDENTITY : 0;
>   }
> 
> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
> +{
> +	int ret;
> +	u32 val;
> +	int i;
> +
> +	ret = arm_mmu500_reset(smmu);
> +	if (ret)
> +		return ret;
> +
> +	/* arm_mmu500_reset() disables CPRE which is re-enabled here */

I still think it would be good to document why we think this is OK, 
given the reasons for disabling CPRE to begin with.

Thanks,
Robin.

> +	for (i = 0; i < smmu->num_context_banks; ++i) {
> +		val = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
> +		val |= CPRE;
> +		arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, val);
> +	}
> +
> +	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,
> @@ -412,7 +442,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,
>   #ifdef CONFIG_ARM_SMMU_QCOM_DEBUG
> @@ -445,7 +475,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.34.1
>
Bibek Kumar Patro Sept. 3, 2024, 12:59 p.m. UTC | #2 
On 8/30/2024 6:23 PM, Robin Murphy wrote:
> On 16/08/2024 6:42 pm, Bibek Kumar Patro wrote:
>> Default MMU-500 reset operation disables context caching in
>> prefetch buffer. It is however expected for context banks using
>> the ACTLR register to retain their prefetch value during reset
>> and runtime suspend.
>>
>> Replace default MMU-500 reset operation with Qualcomm specific reset
>> operation which envelope the default reset operation and re-enables
>> context caching in prefetch buffer for Qualcomm SoCs.
>>
>> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@quicinc.com>
>> ---
>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 36 ++++++++++++++++++++--
>>   1 file changed, 33 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 
>> b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 36c6b36ad4ff..8ac1850b852f 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -16,6 +16,16 @@
>>
>>   #define QCOM_DUMMY_VAL    -1
>>
>> +/*
>> + * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the
>> + * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch
>> + * buffer). The remaining bits are implementation defined and vary 
>> across
>> + * SoCs.
>> + */
>> +
>> +#define CPRE            (1 << 1)
>> +#define CMTLB            (1 << 0)
>> +
>>   static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
>>   {
>>       return container_of(smmu, struct qcom_smmu, smmu);
>> @@ -381,11 +391,31 @@ static int qcom_smmu_def_domain_type(struct 
>> device *dev)
>>       return match ? IOMMU_DOMAIN_IDENTITY : 0;
>>   }
>>
>> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
>> +{
>> +    int ret;
>> +    u32 val;
>> +    int i;
>> +
>> +    ret = arm_mmu500_reset(smmu);
>> +    if (ret)
>> +        return ret;
>> +
>> +    /* arm_mmu500_reset() disables CPRE which is re-enabled here */
> 
> I still think it would be good to document why we think this is OK, 
> given the reasons for disabling CPRE to begin with.
> 

Hi Robin,

Sure, I agree with your point. I’ll elaborate on the comment here to 
document the reason for re-enabling CPRE.

/* The errata for MMU-500 before the r2p2 revision requires CPRE to be
disabled. The arm_mmu500_reset function disables CPRE to accommodate all
RTL revisions. Since all Qualcomm SoCs are on the r2p4 revision, where
the CPRE bit can be enabled, the qcom_smmu500_reset function re-enables
the CPRE bit for the next-page prefetcher to retain the prefetch value
during reset and runtime suspend operations. */

Does this sound good, let me know of your suggestions as well incase.

Thanks & regards,
Bibek

> Thanks,
> Robin.
> 
>> +    for (i = 0; i < smmu->num_context_banks; ++i) {
>> +        val = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
>> +        val |= CPRE;
>> +        arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, val);
>> +    }
>> +
>> +    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,
>> @@ -412,7 +442,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,
>>   #ifdef CONFIG_ARM_SMMU_QCOM_DEBUG
>> @@ -445,7 +475,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.34.1
>>
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 36c6b36ad4ff..8ac1850b852f 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -16,6 +16,16 @@ 

 #define QCOM_DUMMY_VAL	-1

+/*
+ * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the
+ * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch
+ * buffer). The remaining bits are implementation defined and vary across
+ * SoCs.
+ */
+
+#define CPRE			(1 << 1)
+#define CMTLB			(1 << 0)
+
 static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
 {
 	return container_of(smmu, struct qcom_smmu, smmu);
@@ -381,11 +391,31 @@  static int qcom_smmu_def_domain_type(struct device *dev)
 	return match ? IOMMU_DOMAIN_IDENTITY : 0;
 }

+static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
+{
+	int ret;
+	u32 val;
+	int i;
+
+	ret = arm_mmu500_reset(smmu);
+	if (ret)
+		return ret;
+
+	/* arm_mmu500_reset() disables CPRE which is re-enabled here */
+	for (i = 0; i < smmu->num_context_banks; ++i) {
+		val = arm_smmu_cb_read(smmu, i, ARM_SMMU_CB_ACTLR);
+		val |= CPRE;
+		arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_ACTLR, val);
+	}
+
+	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,
@@ -412,7 +442,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,
 #ifdef CONFIG_ARM_SMMU_QCOM_DEBUG
@@ -445,7 +475,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,