[Freedreno] [RFC PATCH 1/9] dt-bindings: arm-smmu: Add missing Qualcomm SMMU compatibles

[Dmitry Baryshkov]

On 22/10/2022 03:59, Krzysztof Kozlowski wrote:
> On 21/10/2022 12:55, Dmitry Baryshkov wrote:
>> Add missing compatibles used for Adreno SMMU on sc7280 and sm8450
>> platforms and for the Qualcomm v2 SMMU used on SDM630 platform.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>> ---
>>   Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> index 9066e6df1ba1..34ee33a62ba5 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
>> @@ -28,6 +28,7 @@ properties:
>>             - enum:
>>                 - qcom,msm8996-smmu-v2
>>                 - qcom,msm8998-smmu-v2
>> +              - qcom,sdm630-smmu-v2
> 
> So qcom,adreno-smmu is not compatible with Adreno? See below.
> 
>>             - const: qcom,smmu-v2
>>   
>>         - description: Qcom SoCs implementing "arm,mmu-500"
>> @@ -48,10 +49,20 @@ properties:
>>                 - qcom,sm8350-smmu-500
>>                 - qcom,sm8450-smmu-500
>>             - const: arm,mmu-500
>> +
>> +      - description: Qcom Adreno GPUs implementing "arm,smmu-500"
>> +        items:
>> +          - enum:
>> +              - qcom,sc7280-smmu-500
>> +              - qcom,sm8250-smmu-500
>> +          - const: qcom,adreno-smmu
>> +          - const: arm,mmu-500
>>         - description: Qcom Adreno GPUs implementing "arm,smmu-v2"
>>           items:
>>             - enum:
>> +              - qcom,msm8996-smmu-v2
>>                 - qcom,sc7180-smmu-v2
>> +              - qcom,sdm630-smmu-v2
> 
> This does not look correct. The same compatible should not be present in
> two different setups.
> 
> If qcom,msm8996-smmu-v2 is compatible with qcom,adreno-smmu, then your
> first hunk is not correct.

Currently the qcom,adreno-smmu compat string is used as a flag, telling 
the kernel that this SMMU instance needs some special setup to work with 
Adreno GPU driver

For example, we have the following compat lists in the existing DT files:
- "qcom,msm8996-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2"
- "qcom,msm8996-smmu-v2", "qcom,smmu-v2" // not handled by arm-qcom-smmu

- "qcom,sdm630-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2"
- "qcom,sdm630-smmu-v2", "qcom,smmu-v2"

- "qcom,sdm845-smmu-v2", "qcom,adreno-smmu", "qcom,smmu-v2"
- "qcom,sdm845-smmu-500", "arm,mmu-500"
- "qcom,sdm845-smmu-v2", "qcom,smmu-v2" // special setup used on Cheza

- "qcom,sm8250-smmu-500", "qcom,adreno-smmu", "arm,mmu-500"
- "qcom,sm8250-smmu-500", "arm,mmu-500"


As we are trying to refactor the IOMMU bindings, what would be your 
recommendation?

To introduce minimal changes, I wanted to have the following lists:
- "qcom,SOC-smmu-500", "qcom,adreno-smmu", "qcom,smmu-500", "arm,mmu-500"

- "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500"

However maybe you would prefer the following model:

- "qcom,SOC-adreno-smmu-500", "qcom,adreno-smmu-500", "arm,mmu-500"
- "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500"


Or:
- "qcom,SOC-smmu-500", "qcom,smmu-500", "arm,mmu-500" + 
'qcom,adreno-smmu' flag/property?


> 
>>                 - qcom,sdm845-smmu-v2
>>             - const: qcom,adreno-smmu
>>             - const: qcom,smmu-v2
> 
> Best regards,
> Krzysztof
> 

-- 
With best wishes
Dmitry