On Wed, Aug 19, 2020 at 10:36:38AM -0700, Rob Clark wrote:
On Wed, Aug 19, 2020 at 10:03 AM Doug Anderson dianders@chromium.org wrote:
Hi,
On Mon, Aug 17, 2020 at 3:03 PM Rob Clark robdclark@gmail.com wrote:
From: Jordan Crouse jcrouse@codeaurora.org
Every Qcom Adreno GPU has an embedded SMMU for its own use. These devices depend on unique features such as split pagetables, different stall/halt requirements and other settings. Identify them with a compatible string so that they can be identified in the arm-smmu implementation specific code.
Signed-off-by: Jordan Crouse jcrouse@codeaurora.org Reviewed-by: Rob Herring robh@kernel.org Signed-off-by: Rob Clark robdclark@chromium.org
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 503160a7b9a0..5ec5d0d691f6 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -40,6 +40,10 @@ properties: - qcom,sm8150-smmu-500 - qcom,sm8250-smmu-500 - const: arm,mmu-500
- description: Qcom Adreno GPUs implementing "arm,smmu-v2"items:- const: qcom,adreno-smmu- const: qcom,smmu-v2I know I'm kinda late to the game, but this seems weird to me, especially given the later patches in the series like:
https://lore.kernel.org/r/20200817220238.603465-19-robdclark@gmail.com
Specifically in that patch you can see that this IOMMU already had a compatible string and we're changing it and throwing away the model-specific string? I'm guessing that you're just trying to make it easier for code to identify the adreno iommu, but it seems like a better way would have been to just add the adreno compatible in the middle, like:
- description: Qcom Adreno GPUs implementing "arm,smmu-v2" items: - enum: - qcom,msm8996-smmu-v2 - qcom,msm8998-smmu-v2 - qcom,sc7180-smmu-v2 - qcom,sdm845-smmu-v2 - const: qcom,adreno-smmu - const: qcom,smmu-v2Then we still have the SoC-specific compatible string in case we need it but we also have the generic one? It also means that we're not deleting the old compatible string...
I did bring up the thing about removing the compat string in an earlier revision of the series.. but then we realized that qcom,sc7180-smmu-v2 was never actually used anywhere.
But I guess we could: compatible = "qcom,sc7180-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2";
I think the SoC specific string is intended for the "other" SMMU that everybody else uses. Rarely would a workaround for that SMMU affect the GPU and vice versa. Since these are the bindings it doesn't hurt to allow for the possibility but I would be surprised if the occasion presented itself.
Jordan
BR, -R
-Doug
- description: Marvell SoCs implementing "arm,mmu-500" items: - const: marvell,ap806-smmu-500-- 2.26.2