Re: [Freedreno] [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis
On 22/06/2023 00:45, Marijn Suijten wrote: Hi! On 2023-01-18 17:16:21, Bryan O'Donoghue wrote: Each compatible has a different set of clocks which are associated with it. Add in the list of clocks for each compatible. So if each set of compatibles have their own unique set of clocks, is there a reason to have so many duplicate then: blocks? I ran into this while preparing for submitting SM6125 DPU and having no clue where to add it. Acked-by: Rob Herring Acked-by: Krzysztof Kozlowski Signed-off-by: Bryan O'Donoghue --- .../display/msm/dsi-controller-main.yaml | 218 -- 1 file changed, 201 insertions(+), 17 deletions(-) [skipped most of the comments] + + - if: + properties: +compatible: + contains: +enum: + - qcom,sc7180-dsi-ctrl + - qcom,sc7280-dsi-ctrl + - qcom,sm8250-dsi-ctrl + - qcom,sm8150-dsi-ctrl + - qcom,sm8250-dsi-ctrl + - qcom,sm8350-dsi-ctrl + - qcom,sm8450-dsi-ctrl + - qcom,sm8550-dsi-ctrl +then: + properties: +clocks: + maxItems: 6 +clock-names: + items: +- const: byte +- const: byte_intf +- const: pixel +- const: core +- const: iface +- const: bus ... and here ... + + - if: + properties: +compatible: + contains: +enum: + - qcom,sdm660-dsi-ctrl +then: + properties: +clocks: + maxItems: 9 +clock-names: + items: +- const: mdp_core +- const: byte +- const: byte_intf +- const: mnoc +- const: iface +- const: bus +- const: core_mmss +- const: pixel +- const: core + + - if: + properties: +compatible: + contains: +enum: + - qcom,sdm845-dsi-ctrl +then: + properties: +clocks: + maxItems: 6 +clock-names: + items: +- const: byte +- const: byte_intf +- const: pixel +- const: core +- const: iface +- const: bus and here, we have *three* identical lists of clocks. Should they (have been) combined? I can send a patch fixing these all if desired! Probably it would be logical to split follow DPU and MDSS schema and split this file into per-SoC compatibles and a generic file. Then it would be easier to review different SoC parts. Regarding reordering of clocks. I think we have 5 different configurations in dsi_cfg.c, but we definitely can optimize the schema. - Marijn + additionalProperties: false examples: -- 2.38.1 -- With best wishes Dmitry
Re: [Freedreno] [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis
Hi! On 2023-01-18 17:16:21, Bryan O'Donoghue wrote: > Each compatible has a different set of clocks which are associated with it. > Add in the list of clocks for each compatible. So if each set of compatibles have their own unique set of clocks, is there a reason to have so many duplicate then: blocks? I ran into this while preparing for submitting SM6125 DPU and having no clue where to add it. > Acked-by: Rob Herring > Acked-by: Krzysztof Kozlowski > Signed-off-by: Bryan O'Donoghue > --- > .../display/msm/dsi-controller-main.yaml | 218 -- > 1 file changed, 201 insertions(+), 17 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > index 35668caa190c4..ad1ba15b74c19 100644 > --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml > @@ -9,9 +9,6 @@ title: Qualcomm Display DSI controller > maintainers: >- Krishna Manikandan > > -allOf: > - - $ref: "../dsi-controller.yaml#" > - > properties: >compatible: > oneOf: > @@ -50,22 +47,23 @@ properties: > maxItems: 1 > >clocks: > -items: > - - description: Display byte clock > - - description: Display byte interface clock > - - description: Display pixel clock > - - description: Display core clock > - - description: Display AHB clock > - - description: Display AXI clock > +description: | > + Several clocks are used, depending on the variant. Typical ones are:: > + - bus:: Display AHB clock. > + - byte:: Display byte clock. > + - byte_intf:: Display byte interface clock. > + - core:: Display core clock. > + - core_mss:: Core MultiMedia SubSystem clock. mm*?? > + - iface:: Display AXI clock. > + - mdp_core:: MDP Core clock. > + - mnoc:: MNOC clock > + - pixel:: Display pixel clock. > +minItems: 3 > +maxItems: 9 > >clock-names: > -items: > - - const: byte > - - const: byte_intf > - - const: pixel > - - const: core > - - const: iface > - - const: bus > +minItems: 3 > +maxItems: 9 > >phys: > maxItems: 1 > @@ -161,6 +159,192 @@ required: >- assigned-clock-parents >- ports > > +allOf: > + - $ref: ../dsi-controller.yaml# > + - if: > + properties: > +compatible: > + contains: > +enum: > + - qcom,apq8064-dsi-ctrl > +then: > + properties: > +clocks: > + maxItems: 7 > +clock-names: > + items: > +- const: iface > +- const: bus > +- const: core_mmss > +- const: src > +- const: byte > +- const: pixel > +- const: core > + > + - if: > + properties: > +compatible: > + contains: > +enum: > + - qcom,msm8916-dsi-ctrl > +then: > + properties: > +clocks: > + maxItems: 6 > +clock-names: > + items: > +- const: mdp_core > +- const: iface > +- const: bus > +- const: byte > +- const: pixel > +- const: core So this... > + > + - if: > + properties: > +compatible: > + contains: > +enum: > + - qcom,msm8953-dsi-ctrl > +then: > + properties: > +clocks: > + maxItems: 6 > +clock-names: > + items: > +- const: mdp_core > +- const: iface > +- const: bus > +- const: byte > +- const: pixel > +- const: core Is the same as the above. Can we merge msm8953 into msm8916 or do you expect differences down the line? > + > + - if: > + properties: > +compatible: > + contains: > +enum: > + - qcom,msm8974-dsi-ctrl > +then: > + properties: > +clocks: > + maxItems: 7 > +clock-names: > + items: > +- const: mdp_core > +- const: iface > +- const: bus > +- const: byte > +- const: pixel > +- const: core > +- const: core_mmss > + > + - if: > + properties: > +compatible: > + contains: > +enum: > + - qcom,msm8996-dsi-ctrl > +then: > + properties: > +clocks: > + maxItems: 7 > +clock-names: > + items: > +- const: mdp_core > +- const: byte > +- const: iface > +- const: bus > +- const: core_mmss > +- const: pixel > +- const: core This could be the same as msm8226/msm8974 if we reorder the entries. > + > + - if: > + properties:
[Freedreno] [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis
Each compatible has a different set of clocks which are associated with it. Add in the list of clocks for each compatible. Acked-by: Rob Herring Acked-by: Krzysztof Kozlowski Signed-off-by: Bryan O'Donoghue --- .../display/msm/dsi-controller-main.yaml | 218 -- 1 file changed, 201 insertions(+), 17 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml index 35668caa190c4..ad1ba15b74c19 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml +++ b/Documentation/devicetree/bindings/display/msm/dsi-controller-main.yaml @@ -9,9 +9,6 @@ title: Qualcomm Display DSI controller maintainers: - Krishna Manikandan -allOf: - - $ref: "../dsi-controller.yaml#" - properties: compatible: oneOf: @@ -50,22 +47,23 @@ properties: maxItems: 1 clocks: -items: - - description: Display byte clock - - description: Display byte interface clock - - description: Display pixel clock - - description: Display core clock - - description: Display AHB clock - - description: Display AXI clock +description: | + Several clocks are used, depending on the variant. Typical ones are:: + - bus:: Display AHB clock. + - byte:: Display byte clock. + - byte_intf:: Display byte interface clock. + - core:: Display core clock. + - core_mss:: Core MultiMedia SubSystem clock. + - iface:: Display AXI clock. + - mdp_core:: MDP Core clock. + - mnoc:: MNOC clock + - pixel:: Display pixel clock. +minItems: 3 +maxItems: 9 clock-names: -items: - - const: byte - - const: byte_intf - - const: pixel - - const: core - - const: iface - - const: bus +minItems: 3 +maxItems: 9 phys: maxItems: 1 @@ -161,6 +159,192 @@ required: - assigned-clock-parents - ports +allOf: + - $ref: ../dsi-controller.yaml# + - if: + properties: +compatible: + contains: +enum: + - qcom,apq8064-dsi-ctrl +then: + properties: +clocks: + maxItems: 7 +clock-names: + items: +- const: iface +- const: bus +- const: core_mmss +- const: src +- const: byte +- const: pixel +- const: core + + - if: + properties: +compatible: + contains: +enum: + - qcom,msm8916-dsi-ctrl +then: + properties: +clocks: + maxItems: 6 +clock-names: + items: +- const: mdp_core +- const: iface +- const: bus +- const: byte +- const: pixel +- const: core + + - if: + properties: +compatible: + contains: +enum: + - qcom,msm8953-dsi-ctrl +then: + properties: +clocks: + maxItems: 6 +clock-names: + items: +- const: mdp_core +- const: iface +- const: bus +- const: byte +- const: pixel +- const: core + + - if: + properties: +compatible: + contains: +enum: + - qcom,msm8974-dsi-ctrl +then: + properties: +clocks: + maxItems: 7 +clock-names: + items: +- const: mdp_core +- const: iface +- const: bus +- const: byte +- const: pixel +- const: core +- const: core_mmss + + - if: + properties: +compatible: + contains: +enum: + - qcom,msm8996-dsi-ctrl +then: + properties: +clocks: + maxItems: 7 +clock-names: + items: +- const: mdp_core +- const: byte +- const: iface +- const: bus +- const: core_mmss +- const: pixel +- const: core + + - if: + properties: +compatible: + contains: +enum: + - qcom,msm8998-dsi-ctrl +then: + properties: +clocks: + maxItems: 6 +clock-names: + items: +- const: byte +- const: byte_intf +- const: pixel +- const: core +- const: iface +- const: bus + + - if: + properties: +compatible: + contains: +enum: + - qcom,sc7180-dsi-ctrl + - qcom,sc7280-dsi-ctrl + - qcom,sm8250-dsi-ctrl + - qcom,sm8150-dsi-ctrl + - qcom,sm8250-dsi-ctrl + - qcom,sm8350-dsi-ctrl + - qcom,sm8450-dsi-ctrl + - qcom,sm8550-dsi-ctrl +then: + properties: +
