diff mbox series

[v12,1/4] dt-bindings: msm: disp: add yaml schemas for DPU bindings

Message ID 1613127000-3015-1-git-send-email-mkrishn@codeaurora.org
State Superseded
Headers show
Series [v12,1/4] dt-bindings: msm: disp: add yaml schemas for DPU bindings | expand

Commit Message

Krishna Manikandan Feb. 12, 2021, 10:49 a.m. UTC
MSM Mobile Display Subsytem (MDSS) encapsulates sub-blocks
like DPU display controller, DSI etc. Add YAML schema
for DPU device tree bindings.

Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>

Changes in v2:
    - Changed dpu to DPU (Sam Ravnborg)
    - Fixed indentation issues (Sam Ravnborg)
    - Added empty line between different properties (Sam Ravnborg)
    - Replaced reference txt files with  their corresponding
      yaml files (Sam Ravnborg)
    - Modified the file to use "|" only when it is
      necessary (Sam Ravnborg)

Changes in v3:
    - Corrected the license used (Rob Herring)
    - Added maxItems for properties (Rob Herring)
    - Dropped generic descriptions (Rob Herring)
    - Added ranges property (Rob Herring)
    - Corrected the indendation (Rob Herring)
    - Added additionalProperties (Rob Herring)
    - Split dsi file into two, one for dsi controller
      and another one for dsi phy per target (Rob Herring)
    - Corrected description for pinctrl-names (Rob Herring)
    - Corrected the examples used in yaml file (Rob Herring)
    - Delete dsi.txt and dpu.txt (Rob Herring)

Changes in v4:
    - Move schema up by one level (Rob Herring)
    - Add patternProperties for mdp node (Rob Herring)
    - Corrected description of some properties (Rob Herring)

Changes in v5:
    - Correct the indentation (Rob Herring)
    - Remove unnecessary description from properties (Rob Herring)
    - Correct the number of interconnect entries (Rob Herring)
    - Add interconnect names for sc7180 (Rob Herring)
    - Add description for ports (Rob Herring)
    - Remove common properties (Rob Herring)
    - Add unevalutatedProperties (Rob Herring)
    - Reference existing dsi controller yaml in the common
      dsi controller file (Rob Herring)
    - Correct the description of clock names to include only the
      clocks that are required (Rob Herring)
    - Remove properties which are already covered under the common
      binding (Rob Herring)
    - Add dsi phy supply nodes which are required for sc7180 and
      sdm845 targets (Rob Herring)
    - Add type ref for syscon-sfpb (Rob Herring)

Changes in v6:
    - Fixed errors during dt_binding_check (Rob Herring)
    - Add maxItems for phys and phys-names (Rob Herring)
    - Use unevaluatedProperties wherever required (Rob Herring)
    - Removed interrupt controller from required properties for
      dsi controller (Rob Herring)
    - Add constraints for dsi-phy reg-names based on the compatible
      phy version (Rob Herring)
    - Add constraints for dsi-phy supply nodes based on the
      compatible phy version (Rob Herring)

Changes in v7:
    - Add default value for qcom,mdss-mdp-transfer-time-us (Rob Herring)
    - Modify the schema for data-lanes (Rob Herring)
    - Split the phy schema into separate schemas based on
      the phy version (Rob Herring)

Changes in v8:
    - Resolve merge conflicts with latest dsi.txt file
    - Include dp yaml change also in the same series

Changes in v9:
    - Combine target specific dsi controller yaml files
      to a single yaml file (Rob Herring)
    - Combine target specific dsi phy yaml files into a
      single yaml file (Rob Herring)
    - Use unevaluatedProperties and additionalProperties
      wherever required
    - Remove duplicate properties from common yaml files

Changes in v10:
    - Split the patch into separate patches for DPU, DSI and
      PHY (Stephen Boyd)
    - Drop unnecessary fullstop (Stephen Boyd)
    - Add newline whereever required (Stephen Boyd)
    - Add description for clock used (Stephen Boyd)
    - Modify the description for interconnect entries  (Stephen Boyd)
    - Drop assigned clock entries as it a generic property (Stephen Boyd)
    - Correct the definition for interrupts (Stephen Boyd)
    - Drop clock names from required properties (Stephen Boyd)
    - Drop labels for display nodes from example (Stephen Boyd)
    - Drop flags from interrupts entries (Stephen Boyd)

Changes in v11:
    - Drop maxItems for clocks (Stephen Boyd)

Changes in v12:
    - Add description for register property (Stephen Boyd)
    - Add maxItems for interrupts (Stephen Boyd)
    - Add description for iommus property (Stephen Boyd)
    - Add description for interconnects (Stephen Boyd)
    - Change display node name to display_controller (Stephen Boyd)
---
 .../bindings/display/msm/dpu-sc7180.yaml           | 212 +++++++++++++++++++++
 .../bindings/display/msm/dpu-sdm845.yaml           | 202 ++++++++++++++++++++
 .../devicetree/bindings/display/msm/dpu.txt        | 141 --------------
 3 files changed, 414 insertions(+), 141 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
 create mode 100644 Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
 delete mode 100644 Documentation/devicetree/bindings/display/msm/dpu.txt

Comments

Stephen Boyd Feb. 26, 2021, 7:45 p.m. UTC | #1
Quoting Krishna Manikandan (2021-02-12 02:49:57)
> MSM Mobile Display Subsytem (MDSS) encapsulates sub-blocks

> like DPU display controller, DSI etc. Add YAML schema

> for DPU device tree bindings.

> 

> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>

> 

> Changes in v2:

>     - Changed dpu to DPU (Sam Ravnborg)

>     - Fixed indentation issues (Sam Ravnborg)

>     - Added empty line between different properties (Sam Ravnborg)

>     - Replaced reference txt files with  their corresponding

>       yaml files (Sam Ravnborg)

>     - Modified the file to use "|" only when it is

>       necessary (Sam Ravnborg)

> 

> Changes in v3:

>     - Corrected the license used (Rob Herring)

>     - Added maxItems for properties (Rob Herring)

>     - Dropped generic descriptions (Rob Herring)

>     - Added ranges property (Rob Herring)

>     - Corrected the indendation (Rob Herring)

>     - Added additionalProperties (Rob Herring)

>     - Split dsi file into two, one for dsi controller

>       and another one for dsi phy per target (Rob Herring)

>     - Corrected description for pinctrl-names (Rob Herring)

>     - Corrected the examples used in yaml file (Rob Herring)

>     - Delete dsi.txt and dpu.txt (Rob Herring)

> 

> Changes in v4:

>     - Move schema up by one level (Rob Herring)

>     - Add patternProperties for mdp node (Rob Herring)

>     - Corrected description of some properties (Rob Herring)

> 

> Changes in v5:

>     - Correct the indentation (Rob Herring)

>     - Remove unnecessary description from properties (Rob Herring)

>     - Correct the number of interconnect entries (Rob Herring)

>     - Add interconnect names for sc7180 (Rob Herring)

>     - Add description for ports (Rob Herring)

>     - Remove common properties (Rob Herring)

>     - Add unevalutatedProperties (Rob Herring)

>     - Reference existing dsi controller yaml in the common

>       dsi controller file (Rob Herring)

>     - Correct the description of clock names to include only the

>       clocks that are required (Rob Herring)

>     - Remove properties which are already covered under the common

>       binding (Rob Herring)

>     - Add dsi phy supply nodes which are required for sc7180 and

>       sdm845 targets (Rob Herring)

>     - Add type ref for syscon-sfpb (Rob Herring)

> 

> Changes in v6:

>     - Fixed errors during dt_binding_check (Rob Herring)

>     - Add maxItems for phys and phys-names (Rob Herring)

>     - Use unevaluatedProperties wherever required (Rob Herring)

>     - Removed interrupt controller from required properties for

>       dsi controller (Rob Herring)

>     - Add constraints for dsi-phy reg-names based on the compatible

>       phy version (Rob Herring)

>     - Add constraints for dsi-phy supply nodes based on the

>       compatible phy version (Rob Herring)

> 

> Changes in v7:

>     - Add default value for qcom,mdss-mdp-transfer-time-us (Rob Herring)

>     - Modify the schema for data-lanes (Rob Herring)

>     - Split the phy schema into separate schemas based on

>       the phy version (Rob Herring)

> 

> Changes in v8:

>     - Resolve merge conflicts with latest dsi.txt file

>     - Include dp yaml change also in the same series

> 

> Changes in v9:

>     - Combine target specific dsi controller yaml files

>       to a single yaml file (Rob Herring)

>     - Combine target specific dsi phy yaml files into a

>       single yaml file (Rob Herring)

>     - Use unevaluatedProperties and additionalProperties

>       wherever required

>     - Remove duplicate properties from common yaml files

> 

> Changes in v10:

>     - Split the patch into separate patches for DPU, DSI and

>       PHY (Stephen Boyd)

>     - Drop unnecessary fullstop (Stephen Boyd)

>     - Add newline whereever required (Stephen Boyd)

>     - Add description for clock used (Stephen Boyd)

>     - Modify the description for interconnect entries  (Stephen Boyd)

>     - Drop assigned clock entries as it a generic property (Stephen Boyd)

>     - Correct the definition for interrupts (Stephen Boyd)

>     - Drop clock names from required properties (Stephen Boyd)

>     - Drop labels for display nodes from example (Stephen Boyd)

>     - Drop flags from interrupts entries (Stephen Boyd)

> 

> Changes in v11:

>     - Drop maxItems for clocks (Stephen Boyd)

> 

> Changes in v12:

>     - Add description for register property (Stephen Boyd)

>     - Add maxItems for interrupts (Stephen Boyd)

>     - Add description for iommus property (Stephen Boyd)

>     - Add description for interconnects (Stephen Boyd)

>     - Change display node name to display_controller (Stephen Boyd)

> ---


Looks mostly good to me. Some minor comments but otherwise seems OK.

> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml

> new file mode 100644

> index 0000000..df88146

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml

> @@ -0,0 +1,212 @@

> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/display/msm/dpu-sc7180.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Description of Qualcomm Display DPU dt properties

> +

> +maintainers:

> +  - Krishna Manikandan <mkrishn@codeaurora.org>

> +

> +description: |

> +  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates

> +  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree

> +  bindings of MDSS and DPU are mentioned for SC7180 target.

> +

> +properties:

> +  compatible:

> +    items:

> +      - const: qcom,sc7180-mdss

> +

> +  reg:

> +    items:

> +      - description: Address offset and size for mdss register set


I think the "rule" is if there's only one reg property then it is
maxItems: 1, sorry about that confusion.

> +

> +  reg-names:

> +    const: mdss

> +

> +  power-domains:

> +    maxItems: 1

> +

> +  clocks:

> +    items:

> +      - description: Display AHB clock from gcc

> +      - description: Display AHB clock from dispcc

> +      - description: Display core clock

> +

> +  clock-names:

> +    items:

> +      - const: iface

> +      - const: ahb

> +      - const: core

> +

> +  interrupts:

> +    maxItems: 1

> +

> +  interrupt-controller: true

> +

> +  "#interrupt-cells":

> +    const: 1

> +

> +  iommus:

> +    items:

> +      - description: Phandle to apps_smmu node with sid mask for hf0

> +

> +  "#address-cells":

> +    const: 2

> +

> +  "#size-cells":

> +    const: 2

> +

> +  ranges: true


Why isn't this part of the example?

> +

> +  interconnects:

> +    items:

> +      - description: Interconnect path specifying the port ids for data bus

> +

> +  interconnect-names:

> +    const: mdp0-mem

> +

> +patternProperties:

> +  "^display-controller@[0-9a-f]+$":

> +    type: object

> +    description: Node containing the properties of DPU.

> +

> +    properties:

> +      compatible:

> +        items:

> +          - const: qcom,sc7180-dpu

> +

> +      reg:

> +        items:

> +          - description: Address offset and size for mdp register set

> +          - description: Address offset and size for vbif register set

> +

> +      reg-names:

> +        items:

> +          - const: mdp

> +          - const: vbif

> +

> +      clocks:

> +        items:

> +          - description: Display hf axi clock

> +          - description: Display ahb clock

> +          - description: Display rotator clock

> +          - description: Display lut clock

> +          - description: Display core clock

> +          - description: Display vsync clock

> +

> +      clock-names:

> +        items:

> +          - const: bus

> +          - const: iface

> +          - const: rot

> +          - const: lut

> +          - const: core

> +          - const: vsync

> +

> +      interrupts:

> +        maxItems: 1

> +

> +      ports:

> +        type: object

> +        description: |

> +          Contains the list of output ports from DPU device. These ports

> +          connect to interfaces that are external to the DPU hardware,

> +          such as DSI, DP etc. Each output port contains an endpoint that

> +          describes how it is connected to an external interface. These

> +          are described by the standard properties documented in files

> +          mentioned below.

> +

> +          Documentation/devicetree/bindings/graph.txt

> +          Documentation/devicetree/bindings/media/video-interfaces.txt

> +

> +        properties:

> +          port@0:

> +            type: object

> +            description: DPU_INTF1 (DSI1)

> +

> +          port@1:

> +            type: object

> +            description: DPU_INTF2 (DSI2)

> +

> +required:

> +  - compatible

> +  - reg

> +  - reg-names

> +  - power-domains

> +  - clocks

> +  - interrupts

> +  - interrupt-controller

> +  - iommus


Shouldn't ranges be required?

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    #include <dt-bindings/clock/qcom,dispcc-sc7180.h>

> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>

> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

> +    #include <dt-bindings/interconnect/qcom,sdm845.h>

> +

> +    soc {

> +      #address-cells = <2>;

> +      #size-cells = <2>;

> +

> +      mdss@ae00000 {

> +         compatible = "qcom,sc7180-mdss";

> +         #address-cells = <2>;

> +         #size-cells = <2>;

> +         reg = <0 0xae00000 0 0x1000>;

> +         reg-names = "mdss";

> +         power-domains = <&dispcc MDSS_GDSC>;

> +

> +         clocks = <&gcc GCC_DISP_AHB_CLK>,

> +                  <&dispcc DISP_CC_MDSS_AHB_CLK>,

> +                  <&dispcc DISP_CC_MDSS_MDP_CLK>;

> +         clock-names = "iface", "ahb", "core";

> +

> +         interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;

> +         interrupt-controller;

> +         #interrupt-cells = <1>;

> +

> +         interconnects = <&mmss_noc MASTER_MDP0 &mc_virt SLAVE_EBI1>;

> +         interconnect-names = "mdp0-mem";

> +

> +         iommus = <&apps_smmu 0x800 0x2>;

> +

> +         display-controller@ae01000 {

> +                   compatible = "qcom,sc7180-dpu";

> +                   reg = <0 0x0ae01000 0 0x8f000>,

> +                         <0 0x0aeb0000 0 0x2008>;

> +

> +                   reg-names = "mdp", "vbif";

> +

> +                   clocks = <&gcc GCC_DISP_HF_AXI_CLK>,

> +                            <&dispcc DISP_CC_MDSS_AHB_CLK>,

> +                            <&dispcc DISP_CC_MDSS_ROT_CLK>,

> +                            <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,

> +                            <&dispcc DISP_CC_MDSS_MDP_CLK>,

> +                            <&dispcc DISP_CC_MDSS_VSYNC_CLK>;

> +                   clock-names = "bus", "iface", "rot", "lut", "core",

> +                                 "vsync";

> +

> +                   interrupt-parent = <&mdss>;

> +                   interrupts = <0>;

> +

> +                   ports {

> +                           #address-cells = <1>;

> +                           #size-cells = <0>;

> +

> +                           port@0 {

> +                                   reg = <0>;

> +                                   dpu_intf1_out: endpoint {

> +                                                  remote-endpoint = <&dsi0_in>;

> +                                   };

> +                           };

> +                   };

> +         };

> +      };

> +    };

> +...

> diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml

> new file mode 100644

> index 0000000..71f8658

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml

> @@ -0,0 +1,202 @@

> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/display/msm/dpu-sdm845.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Description of Qualcomm Display DPU dt properties

> +

> +maintainers:

> +  - Krishna Manikandan <mkrishn@codeaurora.org>

> +

> +description: |

> +  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates

> +  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree

> +  bindings of MDSS and DPU are mentioned for SDM845 target.

> +

> +properties:

> +  compatible:

> +    items:

> +      - const: qcom,sdm845-mdss

> +

> +  reg:

> +    items:

> +      - description: Address offset and size for mdss register set

> +

> +  reg-names:

> +    const: mdss

> +

> +  power-domains:

> +    maxItems: 1

> +

> +  clocks:

> +    items:

> +      - description: Display AHB clock from gcc

> +      - description: Display AXI clock

> +      - description: Display core clock

> +

> +  clock-names:

> +    items:

> +      - const: iface

> +      - const: bus

> +      - const: core

> +

> +  interrupts:

> +    maxItems: 1

> +

> +  interrupt-controller: true

> +

> +  "#interrupt-cells":

> +    const: 1

> +

> +  iommus:

> +    items:

> +      - description: Phandle to apps_smmu node with sid mask for hf0

> +      - description: Phandle to apps_smmu node with sid mask for hf1


What does 'hf' mean here? Can you spell it out? And capitalize SID for
Stream ID?

> +

> +  "#address-cells":

> +    const: 2

> +

> +  "#size-cells":

> +    const: 2

> +

> +  ranges: true

> +

> +patternProperties:

> +  "^display-controller@[0-9a-f]+$":

> +    type: object

> +    description: Node containing the properties of DPU.

> +

> +    properties:

> +      compatible:

> +        items:

> +          - const: qcom,sdm845-dpu

> +

> +      reg:

> +        items:

> +          - description: Address offset and size for mdp register set

> +          - description: Address offset and size for vbif register set

> +

> +      reg-names:

> +        items:

> +          - const: mdp

> +          - const: vbif

> +

> +      clocks:

> +        items:

> +          - description: Display ahb clock

> +          - description: Display axi clock

> +          - description: Display core clock

> +          - description: Display vsync clock

> +

> +      clock-names:

> +        items:

> +          - const: iface

> +          - const: bus

> +          - const: core

> +          - const: vsync

> +

> +      interrupts:

> +        maxItems: 1

> +

> +      ports:

> +        type: object

> +        description: |

> +          Contains the list of output ports from DPU device. These ports

> +          connect to interfaces that are external to the DPU hardware,

> +          such as DSI, DP etc. Each output port contains an endpoint that

> +          describes how it is connected to an external interface. These

> +          are described by the standard properties documented in files

> +          mentioned below.

> +

> +          Documentation/devicetree/bindings/graph.txt

> +          Documentation/devicetree/bindings/media/video-interfaces.txt

> +

> +        properties:

> +          port@0:

> +            type: object

> +            description: DPU_INTF1 (DSI1)

> +

> +          port@1:

> +            type: object

> +            description: DPU_INTF2 (DSI2)

> +

> +required:

> +  - compatible

> +  - reg

> +  - reg-names

> +  - power-domains

> +  - clocks

> +  - interrupts

> +  - interrupt-controller

> +  - iommus

> +

> +additionalProperties: false

> +

> +examples:

> +- |

> +    #include <dt-bindings/clock/qcom,dispcc-sdm845.h>

> +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>

> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

> +

> +    soc {

> +      #address-cells = <2>;

> +      #size-cells = <2>;


I think we can drop the soc node from the examples.

> +

> +      mdss@ae00000 {

> +          compatible = "qcom,sdm845-mdss";

> +          #address-cells = <2>;

> +          #size-cells = <2>;

> +          reg = <0 0x0ae00000 0 0x1000>;

> +          reg-names = "mdss";

> +          power-domains = <&dispcc MDSS_GDSC>;

> +

> +          clocks = <&gcc GCC_DISP_AHB_CLK>,

> +                   <&gcc GCC_DISP_AXI_CLK>,

> +                   <&dispcc DISP_CC_MDSS_MDP_CLK>;

> +          clock-names = "iface", "bus", "core";

> +

> +          interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;

> +          interrupt-controller;

> +          #interrupt-cells = <1>;

> +

> +          iommus = <&apps_smmu 0x880 0x8>,

> +                   <&apps_smmu 0xc80 0x8>;

> +

> +          display-controller@ae01000 {

> +                    compatible = "qcom,sdm845-dpu";

> +                    reg = <0 0x0ae01000 0 0x8f000>,

> +                          <0 0x0aeb0000 0 0x2008>;

> +                    reg-names = "mdp", "vbif";

> +

> +                    clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,

> +                             <&dispcc DISP_CC_MDSS_AXI_CLK>,

> +                             <&dispcc DISP_CC_MDSS_MDP_CLK>,

> +                             <&dispcc DISP_CC_MDSS_VSYNC_CLK>;

> +                    clock-names = "iface", "bus", "core", "vsync";

> +

> +                    interrupt-parent = <&mdss>;

> +                    interrupts = <0>;

> +

> +                    ports {

> +                           #address-cells = <1>;

> +                           #size-cells = <0>;

> +

> +                           port@0 {

> +                                   reg = <0>;

> +                                   dpu_intf1_out: endpoint {

> +                                                  remote-endpoint = <&dsi0_in>;

> +                                   };

> +                           };

> +

> +                           port@1 {

> +                                   reg = <1>;

> +                                   dpu_intf2_out: endpoint {

> +                                                  remote-endpoint = <&dsi1_in>;

> +                                   };

> +                           };

> +                    };

> +          };

> +      };

> +    };

> +...
Stephen Boyd Feb. 26, 2021, 7:52 p.m. UTC | #2
Quoting Krishna Manikandan (2021-02-12 02:49:59)
> Add YAML schema for the device tree bindings for DSI PHY.

> 

> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>

> 

> Changes in v1:

>    - Merge dsi-phy.yaml and dsi-phy-10nm.yaml (Stephen Boyd)

>    - Remove qcom,dsi-phy-regulator-ldo-mode (Stephen Boyd)

>    - Add clock cells properly (Stephen Boyd)

>    - Remove unnecessary decription from clock names (Stephen Boyd)

>    - Add pin names for the supply entries for 10nm phy which is

>      used in sc7180 and sdm845 (Stephen Boyd)

>    - Remove unused header files from examples (Stephen Boyd)

>    - Drop labels for display nodes and correct node name (Stephen Boyd)

> 

> Changes in v2:

>    - Drop maxItems for clock (Stephen Boyd)

>    - Add vdds supply pin information for sdm845 (Stephen Boyd)

>    - Add examples for 14nm, 20nm and 28nm phy yaml files (Stephen Boyd)

>    - Keep child nodes directly under soc node (Stephen Boyd)

> ---

>  .../bindings/display/msm/dsi-phy-10nm.yaml         | 85 +++++++++++++++++++++

>  .../bindings/display/msm/dsi-phy-14nm.yaml         | 83 ++++++++++++++++++++

>  .../bindings/display/msm/dsi-phy-20nm.yaml         | 88 ++++++++++++++++++++++

>  .../bindings/display/msm/dsi-phy-28nm.yaml         | 84 +++++++++++++++++++++


Aren't these bindings largely the same modulo the compatible string? Can
we either combine them into one document or use a common binding base
that is imported into the different nanometer phy binding docs? Look at
Documentation/devicetree/bindings/pinctrl/qcom,tlmm-common.yaml for how
this was done for the qcom pinctrl binding.

From a quick glance, the compatible string and the supply name is
different. Otherwise it's the same? Please combine them together.

>  4 files changed, 340 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-10nm.yaml

>  create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-14nm.yaml

>  create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-20nm.yaml

>  create mode 100644 Documentation/devicetree/bindings/display/msm/dsi-phy-28nm.yaml

>
Stephen Boyd Feb. 26, 2021, 7:54 p.m. UTC | #3
Quoting Krishna Manikandan (2021-02-12 02:50:00)
> Add bindings for Snapdragon DisplayPort controller driver.

> 

> Signed-off-by: Chandan Uddaraju <chandanu@codeaurora.org>

> Signed-off-by: Vara Reddy <varar@codeaurora.org>

> Signed-off-by: Tanmay Shah <tanmay@codeaurora.org>

> Signed-off-by: Kuogee Hsieh <khsieh@codeaurora.org>

> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>

> 

> Changes in V2:

> -Provide details about sel-gpio

> 

> Changes in V4:

> -Provide details about max dp lanes

> -Change the commit text

> 

> Changes in V5:

> -moved dp.txt to yaml file

> 

> Changes in v6:

> - Squash all AUX LUT properties into one pattern Property

> - Make aux-cfg[0-9]-settings properties optional

> - Remove PLL/PHY bindings from DP controller dts

> - Add DP clocks description

> - Remove _clk suffix from clock names

> - Rename pixel clock to stream_pixel

> - Remove redundant bindings (GPIO, PHY, HDCP clock, etc..)

> - Fix indentation

> - Add Display Port as interface of DPU in DPU bindings

>   and add port mapping accordingly.

> 

> Chages in v7:

> - Add dp-controller.yaml file common between multiple SOC

> - Rename dp-sc7180.yaml to dp-controller-sc7180.yaml

> - change compatible string and add SOC name to it.

> - Remove Root clock generator for pixel clock

> - Add assigned-clocks and assigned-clock-parents bindings

> - Remove redundant properties, descriptions and blank lines

> - Add DP port in DPU bindings

> - Update depends-on tag in commit message and rebase change accordingly

> 

> Changes in v8:

> - Add MDSS AHB clock in bindings

> 

> Changes in v9:

> - Remove redundant reg-name property

> - Change assigned-clocks and assigned-clocks-parents counts to 2

> - Use IRQ flags in example dts

> 

> Changes in v10:

> - Change title of this patch as it does not contain PLL bindings anymore

> - Remove redundant properties

> - Remove use of IRQ flag

> - Fix ports property

> 

> Changes in v11:

> - add ports required of both #address-cells and  #size-cells

> - add required operating-points-v2

> - add required #sound-dai-cells

> - add required power-domains

> - update maintainer list

> ---

>  .../bindings/display/msm/dp-controller.yaml        | 152 +++++++++++++++++++++

>  .../bindings/display/msm/dpu-sc7180.yaml           |  10 ++


Can this dpu-sc7180.yaml update be split away from this patch and put
into the patch that introduces that yaml file?

>  2 files changed, 162 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/display/msm/dp-controller.yaml

> 


Otherwise

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Krishna Manikandan March 4, 2021, 12:36 p.m. UTC | #4
On 2021-02-27 01:15, Stephen Boyd wrote:
> Quoting Krishna Manikandan (2021-02-12 02:49:57)

>> MSM Mobile Display Subsytem (MDSS) encapsulates sub-blocks

>> like DPU display controller, DSI etc. Add YAML schema

>> for DPU device tree bindings.

>> 

>> Signed-off-by: Krishna Manikandan <mkrishn@codeaurora.org>

>> 

>> Changes in v2:

>>     - Changed dpu to DPU (Sam Ravnborg)

>>     - Fixed indentation issues (Sam Ravnborg)

>>     - Added empty line between different properties (Sam Ravnborg)

>>     - Replaced reference txt files with  their corresponding

>>       yaml files (Sam Ravnborg)

>>     - Modified the file to use "|" only when it is

>>       necessary (Sam Ravnborg)

>> 

>> Changes in v3:

>>     - Corrected the license used (Rob Herring)

>>     - Added maxItems for properties (Rob Herring)

>>     - Dropped generic descriptions (Rob Herring)

>>     - Added ranges property (Rob Herring)

>>     - Corrected the indendation (Rob Herring)

>>     - Added additionalProperties (Rob Herring)

>>     - Split dsi file into two, one for dsi controller

>>       and another one for dsi phy per target (Rob Herring)

>>     - Corrected description for pinctrl-names (Rob Herring)

>>     - Corrected the examples used in yaml file (Rob Herring)

>>     - Delete dsi.txt and dpu.txt (Rob Herring)

>> 

>> Changes in v4:

>>     - Move schema up by one level (Rob Herring)

>>     - Add patternProperties for mdp node (Rob Herring)

>>     - Corrected description of some properties (Rob Herring)

>> 

>> Changes in v5:

>>     - Correct the indentation (Rob Herring)

>>     - Remove unnecessary description from properties (Rob Herring)

>>     - Correct the number of interconnect entries (Rob Herring)

>>     - Add interconnect names for sc7180 (Rob Herring)

>>     - Add description for ports (Rob Herring)

>>     - Remove common properties (Rob Herring)

>>     - Add unevalutatedProperties (Rob Herring)

>>     - Reference existing dsi controller yaml in the common

>>       dsi controller file (Rob Herring)

>>     - Correct the description of clock names to include only the

>>       clocks that are required (Rob Herring)

>>     - Remove properties which are already covered under the common

>>       binding (Rob Herring)

>>     - Add dsi phy supply nodes which are required for sc7180 and

>>       sdm845 targets (Rob Herring)

>>     - Add type ref for syscon-sfpb (Rob Herring)

>> 

>> Changes in v6:

>>     - Fixed errors during dt_binding_check (Rob Herring)

>>     - Add maxItems for phys and phys-names (Rob Herring)

>>     - Use unevaluatedProperties wherever required (Rob Herring)

>>     - Removed interrupt controller from required properties for

>>       dsi controller (Rob Herring)

>>     - Add constraints for dsi-phy reg-names based on the compatible

>>       phy version (Rob Herring)

>>     - Add constraints for dsi-phy supply nodes based on the

>>       compatible phy version (Rob Herring)

>> 

>> Changes in v7:

>>     - Add default value for qcom,mdss-mdp-transfer-time-us (Rob 

>> Herring)

>>     - Modify the schema for data-lanes (Rob Herring)

>>     - Split the phy schema into separate schemas based on

>>       the phy version (Rob Herring)

>> 

>> Changes in v8:

>>     - Resolve merge conflicts with latest dsi.txt file

>>     - Include dp yaml change also in the same series

>> 

>> Changes in v9:

>>     - Combine target specific dsi controller yaml files

>>       to a single yaml file (Rob Herring)

>>     - Combine target specific dsi phy yaml files into a

>>       single yaml file (Rob Herring)

>>     - Use unevaluatedProperties and additionalProperties

>>       wherever required

>>     - Remove duplicate properties from common yaml files

>> 

>> Changes in v10:

>>     - Split the patch into separate patches for DPU, DSI and

>>       PHY (Stephen Boyd)

>>     - Drop unnecessary fullstop (Stephen Boyd)

>>     - Add newline whereever required (Stephen Boyd)

>>     - Add description for clock used (Stephen Boyd)

>>     - Modify the description for interconnect entries  (Stephen Boyd)

>>     - Drop assigned clock entries as it a generic property (Stephen 

>> Boyd)

>>     - Correct the definition for interrupts (Stephen Boyd)

>>     - Drop clock names from required properties (Stephen Boyd)

>>     - Drop labels for display nodes from example (Stephen Boyd)

>>     - Drop flags from interrupts entries (Stephen Boyd)

>> 

>> Changes in v11:

>>     - Drop maxItems for clocks (Stephen Boyd)

>> 

>> Changes in v12:

>>     - Add description for register property (Stephen Boyd)

>>     - Add maxItems for interrupts (Stephen Boyd)

>>     - Add description for iommus property (Stephen Boyd)

>>     - Add description for interconnects (Stephen Boyd)

>>     - Change display node name to display_controller (Stephen Boyd)

>> ---

> 

> Looks mostly good to me. Some minor comments but otherwise seems OK.

> 

>> diff --git 

>> a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml 

>> b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml

>> new file mode 100644

>> index 0000000..df88146

>> --- /dev/null

>> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml

>> @@ -0,0 +1,212 @@

>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause

>> +%YAML 1.2

>> +---

>> +$id: http://devicetree.org/schemas/display/msm/dpu-sc7180.yaml#

>> +$schema: http://devicetree.org/meta-schemas/core.yaml#

>> +

>> +title: Description of Qualcomm Display DPU dt properties

>> +

>> +maintainers:

>> +  - Krishna Manikandan <mkrishn@codeaurora.org>

>> +

>> +description: |

>> +  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that 

>> encapsulates

>> +  sub-blocks like DPU display controller, DSI and DP interfaces etc. 

>> Device tree

>> +  bindings of MDSS and DPU are mentioned for SC7180 target.

>> +

>> +properties:

>> +  compatible:

>> +    items:

>> +      - const: qcom,sc7180-mdss

>> +

>> +  reg:

>> +    items:

>> +      - description: Address offset and size for mdss register set

> 

> I think the "rule" is if there's only one reg property then it is

> maxItems: 1, sorry about that confusion.

> 

>> +

>> +  reg-names:

>> +    const: mdss

>> +

>> +  power-domains:

>> +    maxItems: 1

>> +

>> +  clocks:

>> +    items:

>> +      - description: Display AHB clock from gcc

>> +      - description: Display AHB clock from dispcc

>> +      - description: Display core clock

>> +

>> +  clock-names:

>> +    items:

>> +      - const: iface

>> +      - const: ahb

>> +      - const: core

>> +

>> +  interrupts:

>> +    maxItems: 1

>> +

>> +  interrupt-controller: true

>> +

>> +  "#interrupt-cells":

>> +    const: 1

>> +

>> +  iommus:

>> +    items:

>> +      - description: Phandle to apps_smmu node with sid mask for hf0

>> +

>> +  "#address-cells":

>> +    const: 2

>> +

>> +  "#size-cells":

>> +    const: 2

>> +

>> +  ranges: true

> 

> Why isn't this part of the example?

> 

>> +

>> +  interconnects:

>> +    items:

>> +      - description: Interconnect path specifying the port ids for 

>> data bus

>> +

>> +  interconnect-names:

>> +    const: mdp0-mem

>> +

>> +patternProperties:

>> +  "^display-controller@[0-9a-f]+$":

>> +    type: object

>> +    description: Node containing the properties of DPU.

>> +

>> +    properties:

>> +      compatible:

>> +        items:

>> +          - const: qcom,sc7180-dpu

>> +

>> +      reg:

>> +        items:

>> +          - description: Address offset and size for mdp register set

>> +          - description: Address offset and size for vbif register 

>> set

>> +

>> +      reg-names:

>> +        items:

>> +          - const: mdp

>> +          - const: vbif

>> +

>> +      clocks:

>> +        items:

>> +          - description: Display hf axi clock

>> +          - description: Display ahb clock

>> +          - description: Display rotator clock

>> +          - description: Display lut clock

>> +          - description: Display core clock

>> +          - description: Display vsync clock

>> +

>> +      clock-names:

>> +        items:

>> +          - const: bus

>> +          - const: iface

>> +          - const: rot

>> +          - const: lut

>> +          - const: core

>> +          - const: vsync

>> +

>> +      interrupts:

>> +        maxItems: 1

>> +

>> +      ports:

>> +        type: object

>> +        description: |

>> +          Contains the list of output ports from DPU device. These 

>> ports

>> +          connect to interfaces that are external to the DPU 

>> hardware,

>> +          such as DSI, DP etc. Each output port contains an endpoint 

>> that

>> +          describes how it is connected to an external interface. 

>> These

>> +          are described by the standard properties documented in 

>> files

>> +          mentioned below.

>> +

>> +          Documentation/devicetree/bindings/graph.txt

>> +          

>> Documentation/devicetree/bindings/media/video-interfaces.txt

>> +

>> +        properties:

>> +          port@0:

>> +            type: object

>> +            description: DPU_INTF1 (DSI1)

>> +

>> +          port@1:

>> +            type: object

>> +            description: DPU_INTF2 (DSI2)

>> +

>> +required:

>> +  - compatible

>> +  - reg

>> +  - reg-names

>> +  - power-domains

>> +  - clocks

>> +  - interrupts

>> +  - interrupt-controller

>> +  - iommus

> 

> Shouldn't ranges be required?

> 

>> +

>> +additionalProperties: false

>> +

>> +examples:

>> +  - |

>> +    #include <dt-bindings/clock/qcom,dispcc-sc7180.h>

>> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>

>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

>> +    #include <dt-bindings/interconnect/qcom,sdm845.h>

>> +

>> +    soc {

>> +      #address-cells = <2>;

>> +      #size-cells = <2>;

>> +

>> +      mdss@ae00000 {

>> +         compatible = "qcom,sc7180-mdss";

>> +         #address-cells = <2>;

>> +         #size-cells = <2>;

>> +         reg = <0 0xae00000 0 0x1000>;

>> +         reg-names = "mdss";

>> +         power-domains = <&dispcc MDSS_GDSC>;

>> +

>> +         clocks = <&gcc GCC_DISP_AHB_CLK>,

>> +                  <&dispcc DISP_CC_MDSS_AHB_CLK>,

>> +                  <&dispcc DISP_CC_MDSS_MDP_CLK>;

>> +         clock-names = "iface", "ahb", "core";

>> +

>> +         interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;

>> +         interrupt-controller;

>> +         #interrupt-cells = <1>;

>> +

>> +         interconnects = <&mmss_noc MASTER_MDP0 &mc_virt SLAVE_EBI1>;

>> +         interconnect-names = "mdp0-mem";

>> +

>> +         iommus = <&apps_smmu 0x800 0x2>;

>> +

>> +         display-controller@ae01000 {

>> +                   compatible = "qcom,sc7180-dpu";

>> +                   reg = <0 0x0ae01000 0 0x8f000>,

>> +                         <0 0x0aeb0000 0 0x2008>;

>> +

>> +                   reg-names = "mdp", "vbif";

>> +

>> +                   clocks = <&gcc GCC_DISP_HF_AXI_CLK>,

>> +                            <&dispcc DISP_CC_MDSS_AHB_CLK>,

>> +                            <&dispcc DISP_CC_MDSS_ROT_CLK>,

>> +                            <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,

>> +                            <&dispcc DISP_CC_MDSS_MDP_CLK>,

>> +                            <&dispcc DISP_CC_MDSS_VSYNC_CLK>;

>> +                   clock-names = "bus", "iface", "rot", "lut", 

>> "core",

>> +                                 "vsync";

>> +

>> +                   interrupt-parent = <&mdss>;

>> +                   interrupts = <0>;

>> +

>> +                   ports {

>> +                           #address-cells = <1>;

>> +                           #size-cells = <0>;

>> +

>> +                           port@0 {

>> +                                   reg = <0>;

>> +                                   dpu_intf1_out: endpoint {

>> +                                                  remote-endpoint = 

>> <&dsi0_in>;

>> +                                   };

>> +                           };

>> +                   };

>> +         };

>> +      };

>> +    };

>> +...

>> diff --git 

>> a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml 

>> b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml

>> new file mode 100644

>> index 0000000..71f8658

>> --- /dev/null

>> +++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml

>> @@ -0,0 +1,202 @@

>> +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause

>> +%YAML 1.2

>> +---

>> +$id: http://devicetree.org/schemas/display/msm/dpu-sdm845.yaml#

>> +$schema: http://devicetree.org/meta-schemas/core.yaml#

>> +

>> +title: Description of Qualcomm Display DPU dt properties

>> +

>> +maintainers:

>> +  - Krishna Manikandan <mkrishn@codeaurora.org>

>> +

>> +description: |

>> +  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that 

>> encapsulates

>> +  sub-blocks like DPU display controller, DSI and DP interfaces etc. 

>> Device tree

>> +  bindings of MDSS and DPU are mentioned for SDM845 target.

>> +

>> +properties:

>> +  compatible:

>> +    items:

>> +      - const: qcom,sdm845-mdss

>> +

>> +  reg:

>> +    items:

>> +      - description: Address offset and size for mdss register set

>> +

>> +  reg-names:

>> +    const: mdss

>> +

>> +  power-domains:

>> +    maxItems: 1

>> +

>> +  clocks:

>> +    items:

>> +      - description: Display AHB clock from gcc

>> +      - description: Display AXI clock

>> +      - description: Display core clock

>> +

>> +  clock-names:

>> +    items:

>> +      - const: iface

>> +      - const: bus

>> +      - const: core

>> +

>> +  interrupts:

>> +    maxItems: 1

>> +

>> +  interrupt-controller: true

>> +

>> +  "#interrupt-cells":

>> +    const: 1

>> +

>> +  iommus:

>> +    items:

>> +      - description: Phandle to apps_smmu node with sid mask for hf0

>> +      - description: Phandle to apps_smmu node with sid mask for hf1

> 

> What does 'hf' mean here? Can you spell it out? And capitalize SID for

> Stream ID?

> 

>> +

>> +  "#address-cells":

>> +    const: 2

>> +

>> +  "#size-cells":

>> +    const: 2

>> +

>> +  ranges: true

>> +

>> +patternProperties:

>> +  "^display-controller@[0-9a-f]+$":

>> +    type: object

>> +    description: Node containing the properties of DPU.

>> +

>> +    properties:

>> +      compatible:

>> +        items:

>> +          - const: qcom,sdm845-dpu

>> +

>> +      reg:

>> +        items:

>> +          - description: Address offset and size for mdp register set

>> +          - description: Address offset and size for vbif register 

>> set

>> +

>> +      reg-names:

>> +        items:

>> +          - const: mdp

>> +          - const: vbif

>> +

>> +      clocks:

>> +        items:

>> +          - description: Display ahb clock

>> +          - description: Display axi clock

>> +          - description: Display core clock

>> +          - description: Display vsync clock

>> +

>> +      clock-names:

>> +        items:

>> +          - const: iface

>> +          - const: bus

>> +          - const: core

>> +          - const: vsync

>> +

>> +      interrupts:

>> +        maxItems: 1

>> +

>> +      ports:

>> +        type: object

>> +        description: |

>> +          Contains the list of output ports from DPU device. These 

>> ports

>> +          connect to interfaces that are external to the DPU 

>> hardware,

>> +          such as DSI, DP etc. Each output port contains an endpoint 

>> that

>> +          describes how it is connected to an external interface. 

>> These

>> +          are described by the standard properties documented in 

>> files

>> +          mentioned below.

>> +

>> +          Documentation/devicetree/bindings/graph.txt

>> +          

>> Documentation/devicetree/bindings/media/video-interfaces.txt

>> +

>> +        properties:

>> +          port@0:

>> +            type: object

>> +            description: DPU_INTF1 (DSI1)

>> +

>> +          port@1:

>> +            type: object

>> +            description: DPU_INTF2 (DSI2)

>> +

>> +required:

>> +  - compatible

>> +  - reg

>> +  - reg-names

>> +  - power-domains

>> +  - clocks

>> +  - interrupts

>> +  - interrupt-controller

>> +  - iommus

>> +

>> +additionalProperties: false

>> +

>> +examples:

>> +- |

>> +    #include <dt-bindings/clock/qcom,dispcc-sdm845.h>

>> +    #include <dt-bindings/clock/qcom,gcc-sdm845.h>

>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>

>> +

>> +    soc {

>> +      #address-cells = <2>;

>> +      #size-cells = <2>;

> 

> I think we can drop the soc node from the examples.

Hi Stephen,

In latest dt schema, there is a rule that we have to specify the address 
and size cells or else it will take default value of 1 for both. If we 
use these default values, dt binding check will throw error as display 
uses 2 address cells and 2 size cells. That's why soc node was added to 
specify the values for mdss node.

Thanks,
Krishna
> 

>> +

>> +      mdss@ae00000 {

>> +          compatible = "qcom,sdm845-mdss";

>> +          #address-cells = <2>;

>> +          #size-cells = <2>;

>> +          reg = <0 0x0ae00000 0 0x1000>;

>> +          reg-names = "mdss";

>> +          power-domains = <&dispcc MDSS_GDSC>;

>> +

>> +          clocks = <&gcc GCC_DISP_AHB_CLK>,

>> +                   <&gcc GCC_DISP_AXI_CLK>,

>> +                   <&dispcc DISP_CC_MDSS_MDP_CLK>;

>> +          clock-names = "iface", "bus", "core";

>> +

>> +          interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;

>> +          interrupt-controller;

>> +          #interrupt-cells = <1>;

>> +

>> +          iommus = <&apps_smmu 0x880 0x8>,

>> +                   <&apps_smmu 0xc80 0x8>;

>> +

>> +          display-controller@ae01000 {

>> +                    compatible = "qcom,sdm845-dpu";

>> +                    reg = <0 0x0ae01000 0 0x8f000>,

>> +                          <0 0x0aeb0000 0 0x2008>;

>> +                    reg-names = "mdp", "vbif";

>> +

>> +                    clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,

>> +                             <&dispcc DISP_CC_MDSS_AXI_CLK>,

>> +                             <&dispcc DISP_CC_MDSS_MDP_CLK>,

>> +                             <&dispcc DISP_CC_MDSS_VSYNC_CLK>;

>> +                    clock-names = "iface", "bus", "core", "vsync";

>> +

>> +                    interrupt-parent = <&mdss>;

>> +                    interrupts = <0>;

>> +

>> +                    ports {

>> +                           #address-cells = <1>;

>> +                           #size-cells = <0>;

>> +

>> +                           port@0 {

>> +                                   reg = <0>;

>> +                                   dpu_intf1_out: endpoint {

>> +                                                  remote-endpoint = 

>> <&dsi0_in>;

>> +                                   };

>> +                           };

>> +

>> +                           port@1 {

>> +                                   reg = <1>;

>> +                                   dpu_intf2_out: endpoint {

>> +                                                  remote-endpoint = 

>> <&dsi1_in>;

>> +                                   };

>> +                           };

>> +                    };

>> +          };

>> +      };

>> +    };

>> +...
Stephen Boyd March 5, 2021, 6:53 a.m. UTC | #5
Quoting mkrishn@codeaurora.org (2021-03-04 04:36:05)
> On 2021-02-27 01:15, Stephen Boyd wrote:

> > Quoting Krishna Manikandan (2021-02-12 02:49:57)

> >> +

> >> +    soc {

> >> +      #address-cells = <2>;

> >> +      #size-cells = <2>;

> > 

> > I think we can drop the soc node from the examples.

> Hi Stephen,

> 

> In latest dt schema, there is a rule that we have to specify the address 

> and size cells or else it will take default value of 1 for both. If we 

> use these default values, dt binding check will throw error as display 

> uses 2 address cells and 2 size cells. That's why soc node was added to 

> specify the values for mdss node.

> 


Do you need to use both cells in the example? Presumably the second cell
is all zero, so it's useless. The example doesn't have to have both
cells in the reg property, that can be fixed up when writing the DT for
a particular SoC.
Krishna Manikandan March 5, 2021, 1:30 p.m. UTC | #6
On 2021-03-05 12:23, Stephen Boyd wrote:
> Quoting mkrishn@codeaurora.org (2021-03-04 04:36:05)

>> On 2021-02-27 01:15, Stephen Boyd wrote:

>> > Quoting Krishna Manikandan (2021-02-12 02:49:57)

>> >> +

>> >> +    soc {

>> >> +      #address-cells = <2>;

>> >> +      #size-cells = <2>;

>> >

>> > I think we can drop the soc node from the examples.

>> Hi Stephen,

>> 

>> In latest dt schema, there is a rule that we have to specify the 

>> address

>> and size cells or else it will take default value of 1 for both. If we

>> use these default values, dt binding check will throw error as display

>> uses 2 address cells and 2 size cells. That's why soc node was added 

>> to

>> specify the values for mdss node.

>> 

> 

> Do you need to use both cells in the example? Presumably the second 

> cell

> is all zero, so it's useless. The example doesn't have to have both

> cells in the reg property, that can be fixed up when writing the DT for

> a particular SoC.


Sure Stephen. I will make the changes in the next patchset.
Thanks,
Krishna
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
new file mode 100644
index 0000000..df88146
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml
@@ -0,0 +1,212 @@ 
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/dpu-sc7180.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Description of Qualcomm Display DPU dt properties
+
+maintainers:
+  - Krishna Manikandan <mkrishn@codeaurora.org>
+
+description: |
+  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates
+  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree
+  bindings of MDSS and DPU are mentioned for SC7180 target.
+
+properties:
+  compatible:
+    items:
+      - const: qcom,sc7180-mdss
+
+  reg:
+    items:
+      - description: Address offset and size for mdss register set
+
+  reg-names:
+    const: mdss
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Display AHB clock from gcc
+      - description: Display AHB clock from dispcc
+      - description: Display core clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: ahb
+      - const: core
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+
+  iommus:
+    items:
+      - description: Phandle to apps_smmu node with sid mask for hf0
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  ranges: true
+
+  interconnects:
+    items:
+      - description: Interconnect path specifying the port ids for data bus
+
+  interconnect-names:
+    const: mdp0-mem
+
+patternProperties:
+  "^display-controller@[0-9a-f]+$":
+    type: object
+    description: Node containing the properties of DPU.
+
+    properties:
+      compatible:
+        items:
+          - const: qcom,sc7180-dpu
+
+      reg:
+        items:
+          - description: Address offset and size for mdp register set
+          - description: Address offset and size for vbif register set
+
+      reg-names:
+        items:
+          - const: mdp
+          - const: vbif
+
+      clocks:
+        items:
+          - description: Display hf axi clock
+          - description: Display ahb clock
+          - description: Display rotator clock
+          - description: Display lut clock
+          - description: Display core clock
+          - description: Display vsync clock
+
+      clock-names:
+        items:
+          - const: bus
+          - const: iface
+          - const: rot
+          - const: lut
+          - const: core
+          - const: vsync
+
+      interrupts:
+        maxItems: 1
+
+      ports:
+        type: object
+        description: |
+          Contains the list of output ports from DPU device. These ports
+          connect to interfaces that are external to the DPU hardware,
+          such as DSI, DP etc. Each output port contains an endpoint that
+          describes how it is connected to an external interface. These
+          are described by the standard properties documented in files
+          mentioned below.
+
+          Documentation/devicetree/bindings/graph.txt
+          Documentation/devicetree/bindings/media/video-interfaces.txt
+
+        properties:
+          port@0:
+            type: object
+            description: DPU_INTF1 (DSI1)
+
+          port@1:
+            type: object
+            description: DPU_INTF2 (DSI2)
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - power-domains
+  - clocks
+  - interrupts
+  - interrupt-controller
+  - iommus
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,dispcc-sc7180.h>
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interconnect/qcom,sdm845.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      mdss@ae00000 {
+         compatible = "qcom,sc7180-mdss";
+         #address-cells = <2>;
+         #size-cells = <2>;
+         reg = <0 0xae00000 0 0x1000>;
+         reg-names = "mdss";
+         power-domains = <&dispcc MDSS_GDSC>;
+
+         clocks = <&gcc GCC_DISP_AHB_CLK>,
+                  <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                  <&dispcc DISP_CC_MDSS_MDP_CLK>;
+         clock-names = "iface", "ahb", "core";
+
+         interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+         interrupt-controller;
+         #interrupt-cells = <1>;
+
+         interconnects = <&mmss_noc MASTER_MDP0 &mc_virt SLAVE_EBI1>;
+         interconnect-names = "mdp0-mem";
+
+         iommus = <&apps_smmu 0x800 0x2>;
+
+         display-controller@ae01000 {
+                   compatible = "qcom,sc7180-dpu";
+                   reg = <0 0x0ae01000 0 0x8f000>,
+                         <0 0x0aeb0000 0 0x2008>;
+
+                   reg-names = "mdp", "vbif";
+
+                   clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
+                            <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                            <&dispcc DISP_CC_MDSS_ROT_CLK>,
+                            <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
+                            <&dispcc DISP_CC_MDSS_MDP_CLK>,
+                            <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+                   clock-names = "bus", "iface", "rot", "lut", "core",
+                                 "vsync";
+
+                   interrupt-parent = <&mdss>;
+                   interrupts = <0>;
+
+                   ports {
+                           #address-cells = <1>;
+                           #size-cells = <0>;
+
+                           port@0 {
+                                   reg = <0>;
+                                   dpu_intf1_out: endpoint {
+                                                  remote-endpoint = <&dsi0_in>;
+                                   };
+                           };
+                   };
+         };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
new file mode 100644
index 0000000..71f8658
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml
@@ -0,0 +1,202 @@ 
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/dpu-sdm845.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Description of Qualcomm Display DPU dt properties
+
+maintainers:
+  - Krishna Manikandan <mkrishn@codeaurora.org>
+
+description: |
+  Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates
+  sub-blocks like DPU display controller, DSI and DP interfaces etc. Device tree
+  bindings of MDSS and DPU are mentioned for SDM845 target.
+
+properties:
+  compatible:
+    items:
+      - const: qcom,sdm845-mdss
+
+  reg:
+    items:
+      - description: Address offset and size for mdss register set
+
+  reg-names:
+    const: mdss
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Display AHB clock from gcc
+      - description: Display AXI clock
+      - description: Display core clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: bus
+      - const: core
+
+  interrupts:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+
+  iommus:
+    items:
+      - description: Phandle to apps_smmu node with sid mask for hf0
+      - description: Phandle to apps_smmu node with sid mask for hf1
+
+  "#address-cells":
+    const: 2
+
+  "#size-cells":
+    const: 2
+
+  ranges: true
+
+patternProperties:
+  "^display-controller@[0-9a-f]+$":
+    type: object
+    description: Node containing the properties of DPU.
+
+    properties:
+      compatible:
+        items:
+          - const: qcom,sdm845-dpu
+
+      reg:
+        items:
+          - description: Address offset and size for mdp register set
+          - description: Address offset and size for vbif register set
+
+      reg-names:
+        items:
+          - const: mdp
+          - const: vbif
+
+      clocks:
+        items:
+          - description: Display ahb clock
+          - description: Display axi clock
+          - description: Display core clock
+          - description: Display vsync clock
+
+      clock-names:
+        items:
+          - const: iface
+          - const: bus
+          - const: core
+          - const: vsync
+
+      interrupts:
+        maxItems: 1
+
+      ports:
+        type: object
+        description: |
+          Contains the list of output ports from DPU device. These ports
+          connect to interfaces that are external to the DPU hardware,
+          such as DSI, DP etc. Each output port contains an endpoint that
+          describes how it is connected to an external interface. These
+          are described by the standard properties documented in files
+          mentioned below.
+
+          Documentation/devicetree/bindings/graph.txt
+          Documentation/devicetree/bindings/media/video-interfaces.txt
+
+        properties:
+          port@0:
+            type: object
+            description: DPU_INTF1 (DSI1)
+
+          port@1:
+            type: object
+            description: DPU_INTF2 (DSI2)
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - power-domains
+  - clocks
+  - interrupts
+  - interrupt-controller
+  - iommus
+
+additionalProperties: false
+
+examples:
+- |
+    #include <dt-bindings/clock/qcom,dispcc-sdm845.h>
+    #include <dt-bindings/clock/qcom,gcc-sdm845.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      mdss@ae00000 {
+          compatible = "qcom,sdm845-mdss";
+          #address-cells = <2>;
+          #size-cells = <2>;
+          reg = <0 0x0ae00000 0 0x1000>;
+          reg-names = "mdss";
+          power-domains = <&dispcc MDSS_GDSC>;
+
+          clocks = <&gcc GCC_DISP_AHB_CLK>,
+                   <&gcc GCC_DISP_AXI_CLK>,
+                   <&dispcc DISP_CC_MDSS_MDP_CLK>;
+          clock-names = "iface", "bus", "core";
+
+          interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
+          interrupt-controller;
+          #interrupt-cells = <1>;
+
+          iommus = <&apps_smmu 0x880 0x8>,
+                   <&apps_smmu 0xc80 0x8>;
+
+          display-controller@ae01000 {
+                    compatible = "qcom,sdm845-dpu";
+                    reg = <0 0x0ae01000 0 0x8f000>,
+                          <0 0x0aeb0000 0 0x2008>;
+                    reg-names = "mdp", "vbif";
+
+                    clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
+                             <&dispcc DISP_CC_MDSS_AXI_CLK>,
+                             <&dispcc DISP_CC_MDSS_MDP_CLK>,
+                             <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
+                    clock-names = "iface", "bus", "core", "vsync";
+
+                    interrupt-parent = <&mdss>;
+                    interrupts = <0>;
+
+                    ports {
+                           #address-cells = <1>;
+                           #size-cells = <0>;
+
+                           port@0 {
+                                   reg = <0>;
+                                   dpu_intf1_out: endpoint {
+                                                  remote-endpoint = <&dsi0_in>;
+                                   };
+                           };
+
+                           port@1 {
+                                   reg = <1>;
+                                   dpu_intf2_out: endpoint {
+                                                  remote-endpoint = <&dsi1_in>;
+                                   };
+                           };
+                    };
+          };
+      };
+    };
+...
diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt
deleted file mode 100644
index 551ae26..0000000
--- a/Documentation/devicetree/bindings/display/msm/dpu.txt
+++ /dev/null
@@ -1,141 +0,0 @@ 
-Qualcomm Technologies, Inc. DPU KMS
-
-Description:
-
-Device tree bindings for MSM Mobile Display Subsytem(MDSS) that encapsulates
-sub-blocks like DPU display controller, DSI and DP interfaces etc.
-The DPU display controller is found in SDM845 SoC.
-
-MDSS:
-Required properties:
-- compatible:  "qcom,sdm845-mdss", "qcom,sc7180-mdss"
-- reg: physical base address and length of contoller's registers.
-- reg-names: register region names. The following region is required:
-  * "mdss"
-- power-domains: a power domain consumer specifier according to
-  Documentation/devicetree/bindings/power/power_domain.txt
-- clocks: list of clock specifiers for clocks needed by the device.
-- clock-names: device clock names, must be in same order as clocks property.
-  The following clocks are required:
-  * "iface"
-  * "bus"
-  * "core"
-- interrupts: interrupt signal from MDSS.
-- interrupt-controller: identifies the node as an interrupt controller.
-- #interrupt-cells: specifies the number of cells needed to encode an interrupt
-  source, should be 1.
-- iommus: phandle of iommu device node.
-- #address-cells: number of address cells for the MDSS children. Should be 1.
-- #size-cells: Should be 1.
-- ranges: parent bus address space is the same as the child bus address space.
-- interconnects : interconnect path specifier for MDSS according to
-  Documentation/devicetree/bindings/interconnect/interconnect.txt. Should be
-  2 paths corresponding to 2 AXI ports.
-- interconnect-names : MDSS will have 2 port names to differentiate between the
-  2 interconnect paths defined with interconnect specifier.
-
-Optional properties:
-- assigned-clocks: list of clock specifiers for clocks needing rate assignment
-- assigned-clock-rates: list of clock frequencies sorted in the same order as
-  the assigned-clocks property.
-
-MDP:
-Required properties:
-- compatible: "qcom,sdm845-dpu", "qcom,sc7180-dpu"
-- reg: physical base address and length of controller's registers.
-- reg-names : register region names. The following region is required:
-  * "mdp"
-  * "vbif"
-- clocks: list of clock specifiers for clocks needed by the device.
-- clock-names: device clock names, must be in same order as clocks property.
-  The following clocks are required.
-  * "bus"
-  * "iface"
-  * "core"
-  * "vsync"
-- interrupts: interrupt line from DPU to MDSS.
-- ports: contains the list of output ports from DPU device. These ports connect
-  to interfaces that are external to the DPU hardware, such as DSI, DP etc.
-
-  Each output port contains an endpoint that describes how it is connected to an
-  external interface. These are described by the standard properties documented
-  here:
-	Documentation/devicetree/bindings/graph.txt
-	Documentation/devicetree/bindings/media/video-interfaces.txt
-
-	Port 0 -> DPU_INTF1 (DSI1)
-	Port 1 -> DPU_INTF2 (DSI2)
-
-Optional properties:
-- assigned-clocks: list of clock specifiers for clocks needing rate assignment
-- assigned-clock-rates: list of clock frequencies sorted in the same order as
-  the assigned-clocks property.
-
-Example:
-
-	mdss: mdss@ae00000 {
-		compatible = "qcom,sdm845-mdss";
-		reg = <0xae00000 0x1000>;
-		reg-names = "mdss";
-
-		power-domains = <&clock_dispcc 0>;
-
-		clocks = <&gcc GCC_DISP_AHB_CLK>, <&gcc GCC_DISP_AXI_CLK>,
-			 <&clock_dispcc DISP_CC_MDSS_MDP_CLK>;
-		clock-names = "iface", "bus", "core";
-
-		assigned-clocks = <&clock_dispcc DISP_CC_MDSS_MDP_CLK>;
-		assigned-clock-rates = <300000000>;
-
-		interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
-		interrupt-controller;
-		#interrupt-cells = <1>;
-
-		interconnects = <&rsc_hlos MASTER_MDP0 &rsc_hlos SLAVE_EBI1>,
-				<&rsc_hlos MASTER_MDP1 &rsc_hlos SLAVE_EBI1>;
-
-		interconnect-names = "mdp0-mem", "mdp1-mem";
-
-		iommus = <&apps_iommu 0>;
-
-		#address-cells = <2>;
-		#size-cells = <1>;
-		ranges = <0 0 0xae00000 0xb2008>;
-
-		mdss_mdp: mdp@ae01000 {
-			compatible = "qcom,sdm845-dpu";
-			reg = <0 0x1000 0x8f000>, <0 0xb0000 0x2008>;
-			reg-names = "mdp", "vbif";
-
-			clocks = <&clock_dispcc DISP_CC_MDSS_AHB_CLK>,
-				 <&clock_dispcc DISP_CC_MDSS_AXI_CLK>,
-				 <&clock_dispcc DISP_CC_MDSS_MDP_CLK>,
-				 <&clock_dispcc DISP_CC_MDSS_VSYNC_CLK>;
-			clock-names = "iface", "bus", "core", "vsync";
-
-			assigned-clocks = <&clock_dispcc DISP_CC_MDSS_MDP_CLK>,
-					  <&clock_dispcc DISP_CC_MDSS_VSYNC_CLK>;
-			assigned-clock-rates = <0 0 300000000 19200000>;
-
-			interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
-
-			ports {
-				#address-cells = <1>;
-				#size-cells = <0>;
-
-				port@0 {
-					reg = <0>;
-					dpu_intf1_out: endpoint {
-						remote-endpoint = <&dsi0_in>;
-					};
-				};
-
-				port@1 {
-					reg = <1>;
-					dpu_intf2_out: endpoint {
-						remote-endpoint = <&dsi1_in>;
-					};
-				};
-			};
-		};
-	};