* [PATCHv2 0/2] iommu/arm-smmu-qcom: Add SC7280 support @ 2021-02-26 9:55 Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan 0 siblings, 2 replies; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-02-26 9:55 UTC (permalink / raw) To: Will Deacon, Robin Murphy, Joerg Roedel, Jordan Crouse, Rob Clark, Akhil P Oommen, Bjorn Andersson Cc: linux-arm-msm, iommu, linux-kernel, linux-arm-kernel Patch 1 adds the sc7280 smmu compatible. Patch 2 moves the adreno smmu check before apss smmu to enable adreno smmu specific implementation. Changes in v2: * Add a comment to make sure this order is not changed in future (Jordan) Sai Prakash Ranjan (2): iommu/arm-smmu-qcom: Add SC7280 SMMU compatible iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) base-commit: 7060377ce06f9cd3ed6274c0f2310463feb5baec -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCHv2 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible 2021-02-26 9:55 [PATCHv2 0/2] iommu/arm-smmu-qcom: Add SC7280 support Sai Prakash Ranjan @ 2021-02-26 9:55 ` Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan 1 sibling, 0 replies; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-02-26 9:55 UTC (permalink / raw) To: Will Deacon, Robin Murphy, Joerg Roedel, Jordan Crouse, Rob Clark, Akhil P Oommen, Bjorn Andersson Cc: linux-arm-msm, iommu, linux-kernel, linux-arm-kernel Add compatible for SC7280 SMMU to use the Qualcomm Technologies, Inc. specific implementation. Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 98b3a1c2a181..bea3ee0dabc2 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -166,6 +166,7 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = { { .compatible = "qcom,mdss" }, { .compatible = "qcom,sc7180-mdss" }, { .compatible = "qcom,sc7180-mss-pil" }, + { .compatible = "qcom,sc7280-mdss" }, { .compatible = "qcom,sc8180x-mdss" }, { .compatible = "qcom,sdm845-mdss" }, { .compatible = "qcom,sdm845-mss-pil" }, @@ -330,6 +331,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu, static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = { { .compatible = "qcom,msm8998-smmu-v2" }, { .compatible = "qcom,sc7180-smmu-500" }, + { .compatible = "qcom,sc7280-smmu-500" }, { .compatible = "qcom,sc8180x-smmu-500" }, { .compatible = "qcom,sdm630-smmu-v2" }, { .compatible = "qcom,sdm845-smmu-500" }, -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 9:55 [PATCHv2 0/2] iommu/arm-smmu-qcom: Add SC7280 support Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible Sai Prakash Ranjan @ 2021-02-26 9:55 ` Sai Prakash Ranjan 2021-02-26 15:51 ` Jordan Crouse ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-02-26 9:55 UTC (permalink / raw) To: Will Deacon, Robin Murphy, Joerg Roedel, Jordan Crouse, Rob Clark, Akhil P Oommen, Bjorn Andersson Cc: linux-arm-msm, iommu, linux-kernel, linux-arm-kernel Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU both implement "arm,mmu-500" in some QTI SoCs and to run through adreno smmu specific implementation such as enabling split pagetables support, we need to match the "qcom,adreno-smmu" compatible first before apss smmu or else we will be running apps smmu implementation for adreno smmu and the additional features for adreno smmu is never set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps and adreno smmu implementing "arm,mmu-500", so the adreno smmu implementation is never reached because the current sequence checks for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that specific impl and we never reach adreno smmu specific implementation. Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> --- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) { const struct device_node *np = smmu->dev->of_node; - if (of_match_node(qcom_smmu_impl_of_match, np)) - return qcom_smmu_create(smmu, &qcom_smmu_impl); - + /* + * Do not change this order of implementation, i.e., first adreno + * smmu impl and then apss smmu since we can have both implementing + * arm,mmu-500 in which case we will miss setting adreno smmu specific + * features if the order is changed. + */ if (of_device_is_compatible(np, "qcom,adreno-smmu")) return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); + if (of_match_node(qcom_smmu_impl_of_match, np)) + return qcom_smmu_create(smmu, &qcom_smmu_impl); + return smmu; } -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan @ 2021-02-26 15:51 ` Jordan Crouse 2021-02-26 17:24 ` Bjorn Andersson 2021-04-19 14:38 ` Bjorn Andersson 2 siblings, 0 replies; 18+ messages in thread From: Jordan Crouse @ 2021-02-26 15:51 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Will Deacon, Akhil P Oommen, iommu, Jordan Crouse, linux-arm-kernel, linux-arm-msm, Robin Murphy, linux-kernel On Fri, Feb 26, 2021 at 03:25:40PM +0530, Sai Prakash Ranjan wrote: > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > both implement "arm,mmu-500" in some QTI SoCs and to run through > adreno smmu specific implementation such as enabling split pagetables > support, we need to match the "qcom,adreno-smmu" compatible first > before apss smmu or else we will be running apps smmu implementation > for adreno smmu and the additional features for adreno smmu is never > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > implementation is never reached because the current sequence checks > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > specific impl and we never reach adreno smmu specific implementation. > > Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> Acked-by: Jordan Crouse <jordan@cosmicpenguin.net> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- > 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) > { > const struct device_node *np = smmu->dev->of_node; > > - if (of_match_node(qcom_smmu_impl_of_match, np)) > - return qcom_smmu_create(smmu, &qcom_smmu_impl); > - > + /* > + * Do not change this order of implementation, i.e., first adreno > + * smmu impl and then apss smmu since we can have both implementing > + * arm,mmu-500 in which case we will miss setting adreno smmu specific > + * features if the order is changed. > + */ > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > + if (of_match_node(qcom_smmu_impl_of_match, np)) > + return qcom_smmu_create(smmu, &qcom_smmu_impl); > + > return smmu; > } > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan 2021-02-26 15:51 ` Jordan Crouse @ 2021-02-26 17:24 ` Bjorn Andersson 2021-02-26 18:23 ` Rob Clark 2021-02-26 18:48 ` Jordan Crouse 2021-04-19 14:38 ` Bjorn Andersson 2 siblings, 2 replies; 18+ messages in thread From: Bjorn Andersson @ 2021-02-26 17:24 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Will Deacon, Akhil P Oommen, iommu, linux-kernel, Jordan Crouse, linux-arm-msm, Robin Murphy, linux-arm-kernel On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > both implement "arm,mmu-500" in some QTI SoCs and to run through > adreno smmu specific implementation such as enabling split pagetables > support, we need to match the "qcom,adreno-smmu" compatible first > before apss smmu or else we will be running apps smmu implementation > for adreno smmu and the additional features for adreno smmu is never > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > implementation is never reached because the current sequence checks > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > specific impl and we never reach adreno smmu specific implementation. > So you're saying that you have a single SMMU instance that's compatible with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"? Per your proposed change we will pick the adreno ops _only_ for this component, essentially disabling the non-Adreno quirks selected by the qcom impl. As such keeping the non-adreno compatible in the qcom_smmu_impl_init[] seems to only serve to obfuscate the situation. Don't we somehow need the combined set of quirks? (At least if we're running this with a standard UEFI based boot flow?) Regards, Bjorn > Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- > 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) > { > const struct device_node *np = smmu->dev->of_node; > > - if (of_match_node(qcom_smmu_impl_of_match, np)) > - return qcom_smmu_create(smmu, &qcom_smmu_impl); > - > + /* > + * Do not change this order of implementation, i.e., first adreno > + * smmu impl and then apss smmu since we can have both implementing > + * arm,mmu-500 in which case we will miss setting adreno smmu specific > + * features if the order is changed. > + */ > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > + if (of_match_node(qcom_smmu_impl_of_match, np)) > + return qcom_smmu_create(smmu, &qcom_smmu_impl); > + > return smmu; > } > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 17:24 ` Bjorn Andersson @ 2021-02-26 18:23 ` Rob Clark 2021-02-26 19:14 ` Bjorn Andersson 2021-02-26 18:48 ` Jordan Crouse 1 sibling, 1 reply; 18+ messages in thread From: Rob Clark @ 2021-02-26 18:23 UTC (permalink / raw) To: Bjorn Andersson Cc: Will Deacon, Akhil P Oommen, list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, , Linux Kernel Mailing List, Jordan Crouse, linux-arm-msm, Robin Murphy, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Fri, Feb 26, 2021 at 9:24 AM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > > both implement "arm,mmu-500" in some QTI SoCs and to run through > > adreno smmu specific implementation such as enabling split pagetables > > support, we need to match the "qcom,adreno-smmu" compatible first > > before apss smmu or else we will be running apps smmu implementation > > for adreno smmu and the additional features for adreno smmu is never > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > > implementation is never reached because the current sequence checks > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > > specific impl and we never reach adreno smmu specific implementation. > > > > So you're saying that you have a single SMMU instance that's compatible > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"? > > Per your proposed change we will pick the adreno ops _only_ for this > component, essentially disabling the non-Adreno quirks selected by the > qcom impl. As such keeping the non-adreno compatible in the > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation. > > Don't we somehow need the combined set of quirks? (At least if we're > running this with a standard UEFI based boot flow?) > are you thinking of the apps-smmu handover of display context bank? That shouldn't change, the only thing that changes is that gpu-smmu becomes an mmu-500, whereas previously only apps-smmu was.. BR, -R _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 18:23 ` Rob Clark @ 2021-02-26 19:14 ` Bjorn Andersson 2021-02-27 13:53 ` Sai Prakash Ranjan 0 siblings, 1 reply; 18+ messages in thread From: Bjorn Andersson @ 2021-02-26 19:14 UTC (permalink / raw) To: Rob Clark Cc: Will Deacon, Akhil P Oommen, list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, , Linux Kernel Mailing List, Jordan Crouse, linux-arm-msm, Robin Murphy, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: > On Fri, Feb 26, 2021 at 9:24 AM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > > > > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > > > both implement "arm,mmu-500" in some QTI SoCs and to run through > > > adreno smmu specific implementation such as enabling split pagetables > > > support, we need to match the "qcom,adreno-smmu" compatible first > > > before apss smmu or else we will be running apps smmu implementation > > > for adreno smmu and the additional features for adreno smmu is never > > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > > > implementation is never reached because the current sequence checks > > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > > > specific impl and we never reach adreno smmu specific implementation. > > > > > > > So you're saying that you have a single SMMU instance that's compatible > > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"? > > > > Per your proposed change we will pick the adreno ops _only_ for this > > component, essentially disabling the non-Adreno quirks selected by the > > qcom impl. As such keeping the non-adreno compatible in the > > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation. > > > > Don't we somehow need the combined set of quirks? (At least if we're > > running this with a standard UEFI based boot flow?) > > > > are you thinking of the apps-smmu handover of display context bank? > That shouldn't change, the only thing that changes is that gpu-smmu > becomes an mmu-500, whereas previously only apps-smmu was.. > The current logic picks one of: 1) is the compatible mentioned in qcom_smmu_impl_of_match[] 2) is the compatible an adreno 3) no quirks needed The change flips the order of these, so the only way I can see this change affecting things is if we expected a match on #2, but we got one on #1. Which implies that the instance that we want to act according to the adreno impl was listed in qcom_smmu_impl_of_match[] - which either is wrong, or there's a single instance that needs both behaviors. (And I believe Jordan's answer confirms the latter - there's a single SMMU instance that needs all them quirks at once) Regards, Bjorn _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 19:14 ` Bjorn Andersson @ 2021-02-27 13:53 ` Sai Prakash Ranjan 2021-03-11 23:29 ` Bjorn Andersson 0 siblings, 1 reply; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-02-27 13:53 UTC (permalink / raw) To: bjorn.andersson Cc: linux-kernel, will, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel Hi Bjorn, On 2021-02-27 00:44, Bjorn Andersson wrote: > On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: > > > The current logic picks one of: > 1) is the compatible mentioned in qcom_smmu_impl_of_match[] > 2) is the compatible an adreno > 3) no quirks needed > > The change flips the order of these, so the only way I can see this > change affecting things is if we expected a match on #2, but we got one > on #1. > > Which implies that the instance that we want to act according to the > adreno impl was listed in qcom_smmu_impl_of_match[] - which either is > wrong, or there's a single instance that needs both behaviors. > > (And I believe Jordan's answer confirms the latter - there's a single > SMMU instance that needs all them quirks at once) > Let me go through the problem statement in case my commit message wasn't clear. There are two SMMUs (APSS and GPU) on SC7280 and both are SMMU500 (ARM SMMU IP). APSS SMMU compatible - ("qcom,sc7280-smmu-500", "arm,mmu-500") GPU SMMU compatible - ("qcom,sc7280-smmu-500", "qcom,adreno-smmu", "arm,mmu-500") Now if we take SC7180 as an example, GPU SMMU was QSMMU(QCOM SMMU IP) and APSS SMMU was SMMU500(ARM SMMU IP). APSS SMMU compatible - ("qcom,sc7180-smmu-500", "arm,mmu-500") GPU SMMU compatible - ("qcom,sc7180-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2") Current code sequence without this patch, if (of_match_node(qcom_smmu_impl_of_match, np)) return qcom_smmu_create(smmu, &qcom_smmu_impl); if (of_device_is_compatible(np, "qcom,adreno-smmu")) return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); Now if we look at the compatible for SC7180, there is no problem because the APSS SMMU will match the one in qcom_smmu_impl_of_match[] and GPU SMMU will match "qcom,adreno-smmu" because the compatible strings are different. But for SC7280, both the APSS SMMU and GPU SMMU compatible("qcom,sc7280-smmu-500") are same. So GPU SMMU will match with the one in qcom_smmu_impl_of_match[] i.e.., "qcom,sc7280-smmu-500" which functionally doesn't cause any problem but we will miss settings for split pagetables which are part of GPU SMMU specific implementation. We can avoid this with yet another new compatible for GPU SMMU something like "qcom,sc7280-adreno-smmu-500" but since we can handle this easily in the driver and since the IPs are same, meaning if there was a hardware quirk required, then we would need to apply to both of them and would this additional compatible be of any help? Coming to the part of quirks now, you are right saying both SMMUs will need to have the same quirks in SC7280 and similar others where both are based on same IPs but those should probably be *hardware quirks* and if they are software based like the S2CR quirk depending on the firmware, then it might not be applicable to both. In case if it is applicable, then as Jordan mentioned, we can add the same quirks in GPU SMMU implementation. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-27 13:53 ` Sai Prakash Ranjan @ 2021-03-11 23:29 ` Bjorn Andersson 2021-03-14 19:01 ` Sai Prakash Ranjan 0 siblings, 1 reply; 18+ messages in thread From: Bjorn Andersson @ 2021-03-11 23:29 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: linux-kernel, will, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel On Sat 27 Feb 07:53 CST 2021, Sai Prakash Ranjan wrote: > Hi Bjorn, > > On 2021-02-27 00:44, Bjorn Andersson wrote: > > On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: > > > > > > The current logic picks one of: > > 1) is the compatible mentioned in qcom_smmu_impl_of_match[] > > 2) is the compatible an adreno > > 3) no quirks needed > > > > The change flips the order of these, so the only way I can see this > > change affecting things is if we expected a match on #2, but we got one > > on #1. > > > > Which implies that the instance that we want to act according to the > > adreno impl was listed in qcom_smmu_impl_of_match[] - which either is > > wrong, or there's a single instance that needs both behaviors. > > > > (And I believe Jordan's answer confirms the latter - there's a single > > SMMU instance that needs all them quirks at once) > > > > Let me go through the problem statement in case my commit message wasn't > clear. There are two SMMUs (APSS and GPU) on SC7280 and both are SMMU500 > (ARM SMMU IP). > > APSS SMMU compatible - ("qcom,sc7280-smmu-500", "arm,mmu-500") > GPU SMMU compatible - ("qcom,sc7280-smmu-500", "qcom,adreno-smmu", "arm,mmu-500") > > Now if we take SC7180 as an example, GPU SMMU was QSMMU(QCOM SMMU IP) > and APSS SMMU was SMMU500(ARM SMMU IP). > > APSS SMMU compatible - ("qcom,sc7180-smmu-500", "arm,mmu-500") > GPU SMMU compatible - ("qcom,sc7180-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2") > > Current code sequence without this patch, > > if (of_match_node(qcom_smmu_impl_of_match, np)) > return qcom_smmu_create(smmu, &qcom_smmu_impl); > > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > Now if we look at the compatible for SC7180, there is no problem because > the APSS SMMU will match the one in qcom_smmu_impl_of_match[] and GPU SMMU > will match "qcom,adreno-smmu" because the compatible strings are different. > But for SC7280, both the APSS SMMU and GPU SMMU compatible("qcom,sc7280-smmu-500") > are same. So GPU SMMU will match with the one in qcom_smmu_impl_of_match[] > i.e.., "qcom,sc7280-smmu-500" which functionally doesn't cause any problem > but we will miss settings for split pagetables which are part of GPU SMMU > specific implementation. > > We can avoid this with yet another new compatible for GPU SMMU something like > "qcom,sc7280-adreno-smmu-500" but since we can handle this easily in the > driver and since the IPs are same, meaning if there was a hardware quirk > required, then we would need to apply to both of them and would this additional > compatible be of any help? > No, I think you're doing the right thing of having them both. I just didn't remember us doing that. > Coming to the part of quirks now, you are right saying both SMMUs will need > to have the same quirks in SC7280 and similar others where both are based on > same IPs but those should probably be *hardware quirks* and if they are > software based like the S2CR quirk depending on the firmware, then it might > not be applicable to both. In case if it is applicable, then as Jordan mentioned, > we can add the same quirks in GPU SMMU implementation. > I do suspect that at some point (probably sooner than later) we'd have to support both inheriting of stream from the bootloader and the Adreno "quirks" in the same instance. But for now this is okay to me. Regards, Bjorn _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-03-11 23:29 ` Bjorn Andersson @ 2021-03-14 19:01 ` Sai Prakash Ranjan 2021-03-25 7:40 ` Sai Prakash Ranjan 0 siblings, 1 reply; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-03-14 19:01 UTC (permalink / raw) To: Bjorn Andersson Cc: linux-kernel, will, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel On 2021-03-12 04:59, Bjorn Andersson wrote: > On Sat 27 Feb 07:53 CST 2021, Sai Prakash Ranjan wrote: > >> Hi Bjorn, >> >> On 2021-02-27 00:44, Bjorn Andersson wrote: >> > On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: >> > >> > >> > The current logic picks one of: >> > 1) is the compatible mentioned in qcom_smmu_impl_of_match[] >> > 2) is the compatible an adreno >> > 3) no quirks needed >> > >> > The change flips the order of these, so the only way I can see this >> > change affecting things is if we expected a match on #2, but we got one >> > on #1. >> > >> > Which implies that the instance that we want to act according to the >> > adreno impl was listed in qcom_smmu_impl_of_match[] - which either is >> > wrong, or there's a single instance that needs both behaviors. >> > >> > (And I believe Jordan's answer confirms the latter - there's a single >> > SMMU instance that needs all them quirks at once) >> > >> >> Let me go through the problem statement in case my commit message >> wasn't >> clear. There are two SMMUs (APSS and GPU) on SC7280 and both are >> SMMU500 >> (ARM SMMU IP). >> >> APSS SMMU compatible - ("qcom,sc7280-smmu-500", "arm,mmu-500") >> GPU SMMU compatible - ("qcom,sc7280-smmu-500", "qcom,adreno-smmu", >> "arm,mmu-500") >> >> Now if we take SC7180 as an example, GPU SMMU was QSMMU(QCOM SMMU IP) >> and APSS SMMU was SMMU500(ARM SMMU IP). >> >> APSS SMMU compatible - ("qcom,sc7180-smmu-500", "arm,mmu-500") >> GPU SMMU compatible - ("qcom,sc7180-smmu-v2", "qcom,adreno-smmu", >> "qcom,smmu-v2") >> >> Current code sequence without this patch, >> >> if (of_match_node(qcom_smmu_impl_of_match, np)) >> return qcom_smmu_create(smmu, &qcom_smmu_impl); >> >> if (of_device_is_compatible(np, "qcom,adreno-smmu")) >> return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); >> >> Now if we look at the compatible for SC7180, there is no problem >> because >> the APSS SMMU will match the one in qcom_smmu_impl_of_match[] and GPU >> SMMU >> will match "qcom,adreno-smmu" because the compatible strings are >> different. >> But for SC7280, both the APSS SMMU and GPU SMMU >> compatible("qcom,sc7280-smmu-500") >> are same. So GPU SMMU will match with the one in >> qcom_smmu_impl_of_match[] >> i.e.., "qcom,sc7280-smmu-500" which functionally doesn't cause any >> problem >> but we will miss settings for split pagetables which are part of GPU >> SMMU >> specific implementation. >> >> We can avoid this with yet another new compatible for GPU SMMU >> something like >> "qcom,sc7280-adreno-smmu-500" but since we can handle this easily in >> the >> driver and since the IPs are same, meaning if there was a hardware >> quirk >> required, then we would need to apply to both of them and would this >> additional >> compatible be of any help? >> > > No, I think you're doing the right thing of having them both. I just > didn't remember us doing that. > >> Coming to the part of quirks now, you are right saying both SMMUs will >> need >> to have the same quirks in SC7280 and similar others where both are >> based on >> same IPs but those should probably be *hardware quirks* and if they >> are >> software based like the S2CR quirk depending on the firmware, then it >> might >> not be applicable to both. In case if it is applicable, then as Jordan >> mentioned, >> we can add the same quirks in GPU SMMU implementation. >> > > I do suspect that at some point (probably sooner than later) we'd have > to support both inheriting of stream from the bootloader and the Adreno > "quirks" in the same instance. > > But for now this is okay to me. > Sure, let me know if you or anyone face any issues without it and I will add it. I will resend this series with the dt-bindings patch for sc7280 smmu which wasn't cc'd to smmu folks by mistake. Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-03-14 19:01 ` Sai Prakash Ranjan @ 2021-03-25 7:40 ` Sai Prakash Ranjan 2021-03-25 15:05 ` Will Deacon 0 siblings, 1 reply; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-03-25 7:40 UTC (permalink / raw) To: Will Deacon Cc: linux-kernel, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel Hi Will, On 2021-03-15 00:31, Sai Prakash Ranjan wrote: > On 2021-03-12 04:59, Bjorn Andersson wrote: >> On Sat 27 Feb 07:53 CST 2021, Sai Prakash Ranjan wrote: >> >>> Hi Bjorn, >>> >>> On 2021-02-27 00:44, Bjorn Andersson wrote: >>> > On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: >>> > >>> > >>> > The current logic picks one of: >>> > 1) is the compatible mentioned in qcom_smmu_impl_of_match[] >>> > 2) is the compatible an adreno >>> > 3) no quirks needed >>> > >>> > The change flips the order of these, so the only way I can see this >>> > change affecting things is if we expected a match on #2, but we got one >>> > on #1. >>> > >>> > Which implies that the instance that we want to act according to the >>> > adreno impl was listed in qcom_smmu_impl_of_match[] - which either is >>> > wrong, or there's a single instance that needs both behaviors. >>> > >>> > (And I believe Jordan's answer confirms the latter - there's a single >>> > SMMU instance that needs all them quirks at once) >>> > >>> >>> Let me go through the problem statement in case my commit message >>> wasn't >>> clear. There are two SMMUs (APSS and GPU) on SC7280 and both are >>> SMMU500 >>> (ARM SMMU IP). >>> >>> APSS SMMU compatible - ("qcom,sc7280-smmu-500", "arm,mmu-500") >>> GPU SMMU compatible - ("qcom,sc7280-smmu-500", "qcom,adreno-smmu", >>> "arm,mmu-500") >>> >>> Now if we take SC7180 as an example, GPU SMMU was QSMMU(QCOM SMMU IP) >>> and APSS SMMU was SMMU500(ARM SMMU IP). >>> >>> APSS SMMU compatible - ("qcom,sc7180-smmu-500", "arm,mmu-500") >>> GPU SMMU compatible - ("qcom,sc7180-smmu-v2", "qcom,adreno-smmu", >>> "qcom,smmu-v2") >>> >>> Current code sequence without this patch, >>> >>> if (of_match_node(qcom_smmu_impl_of_match, np)) >>> return qcom_smmu_create(smmu, &qcom_smmu_impl); >>> >>> if (of_device_is_compatible(np, "qcom,adreno-smmu")) >>> return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); >>> >>> Now if we look at the compatible for SC7180, there is no problem >>> because >>> the APSS SMMU will match the one in qcom_smmu_impl_of_match[] and GPU >>> SMMU >>> will match "qcom,adreno-smmu" because the compatible strings are >>> different. >>> But for SC7280, both the APSS SMMU and GPU SMMU >>> compatible("qcom,sc7280-smmu-500") >>> are same. So GPU SMMU will match with the one in >>> qcom_smmu_impl_of_match[] >>> i.e.., "qcom,sc7280-smmu-500" which functionally doesn't cause any >>> problem >>> but we will miss settings for split pagetables which are part of GPU >>> SMMU >>> specific implementation. >>> >>> We can avoid this with yet another new compatible for GPU SMMU >>> something like >>> "qcom,sc7280-adreno-smmu-500" but since we can handle this easily in >>> the >>> driver and since the IPs are same, meaning if there was a hardware >>> quirk >>> required, then we would need to apply to both of them and would this >>> additional >>> compatible be of any help? >>> >> >> No, I think you're doing the right thing of having them both. I just >> didn't remember us doing that. >> >>> Coming to the part of quirks now, you are right saying both SMMUs >>> will need >>> to have the same quirks in SC7280 and similar others where both are >>> based on >>> same IPs but those should probably be *hardware quirks* and if they >>> are >>> software based like the S2CR quirk depending on the firmware, then it >>> might >>> not be applicable to both. In case if it is applicable, then as >>> Jordan mentioned, >>> we can add the same quirks in GPU SMMU implementation. >>> >> >> I do suspect that at some point (probably sooner than later) we'd have >> to support both inheriting of stream from the bootloader and the >> Adreno >> "quirks" in the same instance. >> >> But for now this is okay to me. >> > > Sure, let me know if you or anyone face any issues without it and I > will > add it. I will resend this series with the dt-bindings patch for sc7280 > smmu > which wasn't cc'd to smmu folks by mistake. > I think there is consensus on this series. I can resend if required but it still applies cleanly, let me know if you have any comments? Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-03-25 7:40 ` Sai Prakash Ranjan @ 2021-03-25 15:05 ` Will Deacon 2021-04-05 8:42 ` Sai Prakash Ranjan 0 siblings, 1 reply; 18+ messages in thread From: Will Deacon @ 2021-03-25 15:05 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: linux-kernel, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel On Thu, Mar 25, 2021 at 01:10:12PM +0530, Sai Prakash Ranjan wrote: > On 2021-03-15 00:31, Sai Prakash Ranjan wrote: > > On 2021-03-12 04:59, Bjorn Andersson wrote: > > > On Sat 27 Feb 07:53 CST 2021, Sai Prakash Ranjan wrote: > > > > On 2021-02-27 00:44, Bjorn Andersson wrote: > > > > > On Fri 26 Feb 12:23 CST 2021, Rob Clark wrote: > > > > > > > > > > > > > > > The current logic picks one of: > > > > > 1) is the compatible mentioned in qcom_smmu_impl_of_match[] > > > > > 2) is the compatible an adreno > > > > > 3) no quirks needed > > > > > > > > > > The change flips the order of these, so the only way I can see this > > > > > change affecting things is if we expected a match on #2, but we got one > > > > > on #1. > > > > > > > > > > Which implies that the instance that we want to act according to the > > > > > adreno impl was listed in qcom_smmu_impl_of_match[] - which either is > > > > > wrong, or there's a single instance that needs both behaviors. > > > > > > > > > > (And I believe Jordan's answer confirms the latter - there's a single > > > > > SMMU instance that needs all them quirks at once) > > > > > > > > > > > > > Let me go through the problem statement in case my commit > > > > message wasn't > > > > clear. There are two SMMUs (APSS and GPU) on SC7280 and both are > > > > SMMU500 > > > > (ARM SMMU IP). > > > > > > > > APSS SMMU compatible - ("qcom,sc7280-smmu-500", "arm,mmu-500") > > > > GPU SMMU compatible - ("qcom,sc7280-smmu-500", > > > > "qcom,adreno-smmu", "arm,mmu-500") > > > > > > > > Now if we take SC7180 as an example, GPU SMMU was QSMMU(QCOM SMMU IP) > > > > and APSS SMMU was SMMU500(ARM SMMU IP). > > > > > > > > APSS SMMU compatible - ("qcom,sc7180-smmu-500", "arm,mmu-500") > > > > GPU SMMU compatible - ("qcom,sc7180-smmu-v2", > > > > "qcom,adreno-smmu", "qcom,smmu-v2") > > > > > > > > Current code sequence without this patch, > > > > > > > > if (of_match_node(qcom_smmu_impl_of_match, np)) > > > > return qcom_smmu_create(smmu, &qcom_smmu_impl); > > > > > > > > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > > > > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > > > > > > > Now if we look at the compatible for SC7180, there is no problem > > > > because > > > > the APSS SMMU will match the one in qcom_smmu_impl_of_match[] > > > > and GPU SMMU > > > > will match "qcom,adreno-smmu" because the compatible strings are > > > > different. > > > > But for SC7280, both the APSS SMMU and GPU SMMU > > > > compatible("qcom,sc7280-smmu-500") > > > > are same. So GPU SMMU will match with the one in > > > > qcom_smmu_impl_of_match[] > > > > i.e.., "qcom,sc7280-smmu-500" which functionally doesn't cause > > > > any problem > > > > but we will miss settings for split pagetables which are part of > > > > GPU SMMU > > > > specific implementation. > > > > > > > > We can avoid this with yet another new compatible for GPU SMMU > > > > something like > > > > "qcom,sc7280-adreno-smmu-500" but since we can handle this > > > > easily in the > > > > driver and since the IPs are same, meaning if there was a > > > > hardware quirk > > > > required, then we would need to apply to both of them and would > > > > this additional > > > > compatible be of any help? > > > > > > > > > > No, I think you're doing the right thing of having them both. I just > > > didn't remember us doing that. > > > > > > > Coming to the part of quirks now, you are right saying both > > > > SMMUs will need > > > > to have the same quirks in SC7280 and similar others where both > > > > are based on > > > > same IPs but those should probably be *hardware quirks* and if > > > > they are > > > > software based like the S2CR quirk depending on the firmware, > > > > then it might > > > > not be applicable to both. In case if it is applicable, then as > > > > Jordan mentioned, > > > > we can add the same quirks in GPU SMMU implementation. > > > > > > > > > > I do suspect that at some point (probably sooner than later) we'd have > > > to support both inheriting of stream from the bootloader and the > > > Adreno > > > "quirks" in the same instance. > > > > > > But for now this is okay to me. > > > > > > > Sure, let me know if you or anyone face any issues without it and I will > > add it. I will resend this series with the dt-bindings patch for sc7280 > > smmu > > which wasn't cc'd to smmu folks by mistake. > > > > I think there is consensus on this series. I can resend if required but it > still applies cleanly, let me know if you have any comments? Please resend with the bindings patch, and I'd like Bjorn's Ack as well. Will _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-03-25 15:05 ` Will Deacon @ 2021-04-05 8:42 ` Sai Prakash Ranjan 2021-04-19 3:11 ` Sai Prakash Ranjan 0 siblings, 1 reply; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-04-05 8:42 UTC (permalink / raw) To: Will Deacon, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel Hi Bjorn, On 2021-03-25 20:35, Will Deacon wrote: > On Thu, Mar 25, 2021 at 01:10:12PM +0530, Sai Prakash Ranjan wrote: <snip>... >> >> I think there is consensus on this series. I can resend if required >> but it >> still applies cleanly, let me know if you have any comments? > > Please resend with the bindings patch, and I'd like Bjorn's Ack as > well. > Can we have your review/ack in case there is nothing pending here? Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-04-05 8:42 ` Sai Prakash Ranjan @ 2021-04-19 3:11 ` Sai Prakash Ranjan 0 siblings, 0 replies; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-04-19 3:11 UTC (permalink / raw) To: Will Deacon, Bjorn Andersson Cc: linux-kernel, linux-arm-msm, jcrouse, akhilpo, iommu, robin.murphy, linux-arm-kernel On 2021-04-05 14:12, Sai Prakash Ranjan wrote: > Hi Bjorn, > > On 2021-03-25 20:35, Will Deacon wrote: >> On Thu, Mar 25, 2021 at 01:10:12PM +0530, Sai Prakash Ranjan wrote: > > <snip>... > >>> >>> I think there is consensus on this series. I can resend if required >>> but it >>> still applies cleanly, let me know if you have any comments? >> >> Please resend with the bindings patch, and I'd like Bjorn's Ack as >> well. >> > > Can we have your review/ack in case there is nothing pending here? > Gentle Ping! Thanks, Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 17:24 ` Bjorn Andersson 2021-02-26 18:23 ` Rob Clark @ 2021-02-26 18:48 ` Jordan Crouse 2021-02-26 19:54 ` Jordan Crouse 1 sibling, 1 reply; 18+ messages in thread From: Jordan Crouse @ 2021-02-26 18:48 UTC (permalink / raw) To: Bjorn Andersson Cc: linux-arm-msm, Akhil P Oommen, iommu, linux-kernel, Jordan Crouse, Will Deacon, linux-arm-kernel, Robin Murphy On Fri, Feb 26, 2021 at 11:24:52AM -0600, Bjorn Andersson wrote: > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > > both implement "arm,mmu-500" in some QTI SoCs and to run through > > adreno smmu specific implementation such as enabling split pagetables > > support, we need to match the "qcom,adreno-smmu" compatible first > > before apss smmu or else we will be running apps smmu implementation > > for adreno smmu and the additional features for adreno smmu is never > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > > implementation is never reached because the current sequence checks > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > > specific impl and we never reach adreno smmu specific implementation. > > > > So you're saying that you have a single SMMU instance that's compatible > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"? > > Per your proposed change we will pick the adreno ops _only_ for this > component, essentially disabling the non-Adreno quirks selected by the > qcom impl. As such keeping the non-adreno compatible in the > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation. > > Don't we somehow need the combined set of quirks? (At least if we're > running this with a standard UEFI based boot flow?) We *do* need the combined set of quirks, so there has to be an adreno-smmu impelmentation that matches the "generic" implementation with a few extra function hooks added on. I'm not sure if there is a clever way to figure out how to meld the implementation hooks at runtime but the alternative is to just make sure that the adreno-smmu static struct calls the same quirks as its generic partner. Jordan > > Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > > --- > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- > > 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) > > { > > const struct device_node *np = smmu->dev->of_node; > > > > - if (of_match_node(qcom_smmu_impl_of_match, np)) > > - return qcom_smmu_create(smmu, &qcom_smmu_impl); > > - > > + /* > > + * Do not change this order of implementation, i.e., first adreno > > + * smmu impl and then apss smmu since we can have both implementing > > + * arm,mmu-500 in which case we will miss setting adreno smmu specific > > + * features if the order is changed. > > + */ > > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > > > + if (of_match_node(qcom_smmu_impl_of_match, np)) > > + return qcom_smmu_create(smmu, &qcom_smmu_impl); > > + > > return smmu; > > } > > -- > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > > of Code Aurora Forum, hosted by The Linux Foundation > > > _______________________________________________ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 18:48 ` Jordan Crouse @ 2021-02-26 19:54 ` Jordan Crouse 0 siblings, 0 replies; 18+ messages in thread From: Jordan Crouse @ 2021-02-26 19:54 UTC (permalink / raw) To: Bjorn Andersson, Sai Prakash Ranjan, Will Deacon, Akhil P Oommen, iommu, linux-kernel, Jordan Crouse, linux-arm-msm, Robin Murphy, linux-arm-kernel On Fri, Feb 26, 2021 at 11:48:13AM -0700, Jordan Crouse wrote: > On Fri, Feb 26, 2021 at 11:24:52AM -0600, Bjorn Andersson wrote: > > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > > > > > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > > > both implement "arm,mmu-500" in some QTI SoCs and to run through > > > adreno smmu specific implementation such as enabling split pagetables > > > support, we need to match the "qcom,adreno-smmu" compatible first > > > before apss smmu or else we will be running apps smmu implementation > > > for adreno smmu and the additional features for adreno smmu is never > > > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > > > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > > > implementation is never reached because the current sequence checks > > > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > > > specific impl and we never reach adreno smmu specific implementation. > > > > > > > So you're saying that you have a single SMMU instance that's compatible > > with both an entry in qcom_smmu_impl_of_match[] and "qcom,adreno-smmu"? > > > > Per your proposed change we will pick the adreno ops _only_ for this > > component, essentially disabling the non-Adreno quirks selected by the > > qcom impl. As such keeping the non-adreno compatible in the > > qcom_smmu_impl_init[] seems to only serve to obfuscate the situation. > > > > Don't we somehow need the combined set of quirks? (At least if we're > > running this with a standard UEFI based boot flow?) > > We *do* need the combined set of quirks, so there has to be an adreno-smmu > impelmentation that matches the "generic" implementation with a few extra > function hooks added on. I'm not sure if there is a clever way to figure out how > to meld the implementation hooks at runtime but the alternative is to just make > sure that the adreno-smmu static struct calls the same quirks as its generic > partner. To clarify, the gpu-smmu doesn't strictly need the s2cr handoff or the cfg_probe though it wouldn't hurt to have them since they would be essentially passthroughs for the GPU. We do need to capture errata like the sdm845_smmu500_reset which is already part of the upstream adreno implementation. I think the main takeaway is that if a new errata or quirk is added for main mmu500 it needs to be considered for adreno-smmu too. Jordan > > > Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> > > > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > > > --- > > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- > > > 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 > > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) > > > { > > > const struct device_node *np = smmu->dev->of_node; > > > > > > - if (of_match_node(qcom_smmu_impl_of_match, np)) > > > - return qcom_smmu_create(smmu, &qcom_smmu_impl); > > > - > > > + /* > > > + * Do not change this order of implementation, i.e., first adreno > > > + * smmu impl and then apss smmu since we can have both implementing > > > + * arm,mmu-500 in which case we will miss setting adreno smmu specific > > > + * features if the order is changed. > > > + */ > > > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > > > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > > > > > + if (of_match_node(qcom_smmu_impl_of_match, np)) > > > + return qcom_smmu_create(smmu, &qcom_smmu_impl); > > > + > > > return smmu; > > > } > > > -- > > > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > > > of Code Aurora Forum, hosted by The Linux Foundation > > > > > _______________________________________________ > > iommu mailing list > > iommu@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/iommu _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan 2021-02-26 15:51 ` Jordan Crouse 2021-02-26 17:24 ` Bjorn Andersson @ 2021-04-19 14:38 ` Bjorn Andersson 2021-04-20 6:06 ` Sai Prakash Ranjan 2 siblings, 1 reply; 18+ messages in thread From: Bjorn Andersson @ 2021-04-19 14:38 UTC (permalink / raw) To: Sai Prakash Ranjan Cc: Will Deacon, Akhil P Oommen, iommu, linux-kernel, Jordan Crouse, linux-arm-msm, Robin Murphy, linux-arm-kernel On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU > both implement "arm,mmu-500" in some QTI SoCs and to run through > adreno smmu specific implementation such as enabling split pagetables > support, we need to match the "qcom,adreno-smmu" compatible first > before apss smmu or else we will be running apps smmu implementation > for adreno smmu and the additional features for adreno smmu is never > set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps > and adreno smmu implementing "arm,mmu-500", so the adreno smmu > implementation is never reached because the current sequence checks > for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that > specific impl and we never reach adreno smmu specific implementation. > > Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> > Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> Sorry for taking my time thinking about this. Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 12 +++++++++--- > 1 file changed, 9 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 bea3ee0dabc2..03f048aebb80 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -345,11 +345,17 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu) > { > const struct device_node *np = smmu->dev->of_node; > > - if (of_match_node(qcom_smmu_impl_of_match, np)) > - return qcom_smmu_create(smmu, &qcom_smmu_impl); > - > + /* > + * Do not change this order of implementation, i.e., first adreno > + * smmu impl and then apss smmu since we can have both implementing > + * arm,mmu-500 in which case we will miss setting adreno smmu specific > + * features if the order is changed. > + */ > if (of_device_is_compatible(np, "qcom,adreno-smmu")) > return qcom_smmu_create(smmu, &qcom_adreno_smmu_impl); > > + if (of_match_node(qcom_smmu_impl_of_match, np)) > + return qcom_smmu_create(smmu, &qcom_smmu_impl); > + > return smmu; > } > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier 2021-04-19 14:38 ` Bjorn Andersson @ 2021-04-20 6:06 ` Sai Prakash Ranjan 0 siblings, 0 replies; 18+ messages in thread From: Sai Prakash Ranjan @ 2021-04-20 6:06 UTC (permalink / raw) To: Bjorn Andersson Cc: Will Deacon, Akhil P Oommen, iommu, linux-kernel, Jordan Crouse, linux-arm-msm, Robin Murphy, linux-arm-kernel On 2021-04-19 20:08, Bjorn Andersson wrote: > On Fri 26 Feb 03:55 CST 2021, Sai Prakash Ranjan wrote: > >> Adreno(GPU) SMMU and APSS(Application Processor SubSystem) SMMU >> both implement "arm,mmu-500" in some QTI SoCs and to run through >> adreno smmu specific implementation such as enabling split pagetables >> support, we need to match the "qcom,adreno-smmu" compatible first >> before apss smmu or else we will be running apps smmu implementation >> for adreno smmu and the additional features for adreno smmu is never >> set. For ex: we have "qcom,sc7280-smmu-500" compatible for both apps >> and adreno smmu implementing "arm,mmu-500", so the adreno smmu >> implementation is never reached because the current sequence checks >> for apps smmu compatible(qcom,sc7280-smmu-500) first and runs that >> specific impl and we never reach adreno smmu specific implementation. >> >> Suggested-by: Akhil P Oommen <akhilpo@codeaurora.org> >> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org> > > Sorry for taking my time thinking about this. > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > No worries, thanks Bjorn. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-04-20 6:06 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-26 9:55 [PATCHv2 0/2] iommu/arm-smmu-qcom: Add SC7280 support Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 1/2] iommu/arm-smmu-qcom: Add SC7280 SMMU compatible Sai Prakash Ranjan 2021-02-26 9:55 ` [PATCHv2 2/2] iommu/arm-smmu-qcom: Move the adreno smmu specific impl earlier Sai Prakash Ranjan 2021-02-26 15:51 ` Jordan Crouse 2021-02-26 17:24 ` Bjorn Andersson 2021-02-26 18:23 ` Rob Clark 2021-02-26 19:14 ` Bjorn Andersson 2021-02-27 13:53 ` Sai Prakash Ranjan 2021-03-11 23:29 ` Bjorn Andersson 2021-03-14 19:01 ` Sai Prakash Ranjan 2021-03-25 7:40 ` Sai Prakash Ranjan 2021-03-25 15:05 ` Will Deacon 2021-04-05 8:42 ` Sai Prakash Ranjan 2021-04-19 3:11 ` Sai Prakash Ranjan 2021-02-26 18:48 ` Jordan Crouse 2021-02-26 19:54 ` Jordan Crouse 2021-04-19 14:38 ` Bjorn Andersson 2021-04-20 6:06 ` Sai Prakash Ranjan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).