[PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
From: Jordan Crouse 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 Reviewed-by: Rob Herring Signed-off-by: Rob Clark --- Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml index 503160a7b9a0..3b63f2ae24db 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml @@ -28,8 +28,6 @@ properties: - enum: - qcom,msm8996-smmu-v2 - qcom,msm8998-smmu-v2 - - qcom,sc7180-smmu-v2 - - qcom,sdm845-smmu-v2 - const: qcom,smmu-v2 - description: Qcom SoCs implementing "arm,mmu-500" @@ -40,6 +38,13 @@ properties: - qcom,sm8150-smmu-500 - qcom,sm8250-smmu-500 - const: arm,mmu-500 + - description: Qcom Adreno GPUs implementing "arm,smmu-v2" +items: + - enum: + - qcom,sc7180-smmu-v2 + - qcom,sdm845-smmu-v2 + - const: qcom,adreno-smmu + - const: qcom,smmu-v2 - description: Marvell SoCs implementing "arm,mmu-500" items: - const: marvell,ap806-smmu-500 -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
On Wed, Aug 19, 2020 at 10:36:38AM -0700, Rob Clark wrote: > On Wed, Aug 19, 2020 at 10:03 AM Doug Anderson wrote: > > > > Hi, > > > > On Mon, Aug 17, 2020 at 3:03 PM Rob Clark wrote: > > > > > > From: Jordan Crouse > > > > > > 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 > > > Reviewed-by: Rob Herring > > > Signed-off-by: Rob Clark > > > --- > > > 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-v2 > > > > I 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-robdcl...@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-v2 > > > > Then 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 > > > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
Hi, On Wed, Aug 19, 2020 at 10:36 AM Rob Clark wrote: > > On Wed, Aug 19, 2020 at 10:03 AM Doug Anderson wrote: > > > > Hi, > > > > On Mon, Aug 17, 2020 at 3:03 PM Rob Clark wrote: > > > > > > From: Jordan Crouse > > > > > > 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 > > > Reviewed-by: Rob Herring > > > Signed-off-by: Rob Clark > > > --- > > > 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-v2 > > > > I 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-robdcl...@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-v2 > > > > Then 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. Right, so at least there's not going to be weird issues where landing the dts before the code change will break anything. > But I guess we could: compatible = "qcom,sc7180-smmu-v2", > "qcom,adreno-smmu", "qcom,smmu-v2"; Yeah, that was what I was suggesting. -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
On Wed, Aug 19, 2020 at 10:03 AM Doug Anderson wrote: > > Hi, > > On Mon, Aug 17, 2020 at 3:03 PM Rob Clark wrote: > > > > From: Jordan Crouse > > > > 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 > > Reviewed-by: Rob Herring > > Signed-off-by: Rob Clark > > --- > > 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-v2 > > I 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-robdcl...@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-v2 > > Then 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"; BR, -R > > -Doug > > > >- description: Marvell SoCs implementing "arm,mmu-500" > > items: > >- const: marvell,ap806-smmu-500 > > -- > > 2.26.2 > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
Hi, On Mon, Aug 17, 2020 at 3:03 PM Rob Clark wrote: > > From: Jordan Crouse > > 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 > Reviewed-by: Rob Herring > Signed-off-by: Rob Clark > --- > 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-v2 I 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-robdcl...@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-v2 Then 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... -Doug >- description: Marvell SoCs implementing "arm,mmu-500" > items: >- const: marvell,ap806-smmu-500 > -- > 2.26.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/20] dt-bindings: arm-smmu: Add compatible string for Adreno GPU SMMU
From: Jordan Crouse 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 Reviewed-by: Rob Herring Signed-off-by: Rob Clark --- 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-v2 - description: Marvell SoCs implementing "arm,mmu-500" items: - const: marvell,ap806-smmu-500 -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
