diff mbox series

[RFC,01/15] dt-bindings: display: bridge: Add binding for R-Car MIPI DSI/CSI-2 TX

Message ID 20210623034656.10316-2-laurent.pinchart+renesas@ideasonboard.com (mailing list archive)
State New
Delegated to: Kieran Bingham
Headers show
Series R-Car V3U: DSI encoder driver | expand

Commit Message

Laurent Pinchart June 23, 2021, 3:46 a.m. UTC 
The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
can operate in either DSI or CSI-2 mode, with up to four data lanes.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml

Comments

Kieran Bingham June 23, 2021, 9:06 a.m. UTC | #1 
Hi Laurent,

On 23/06/2021 04:46, Laurent Pinchart wrote:
> The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
> can operate in either DSI or CSI-2 mode, with up to four data lanes.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++++++++++++++++++
>  1 file changed, 118 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> new file mode 100644
> index 000000000000..7e1b606a65ea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> @@ -0,0 +1,118 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/renesas,dsi-csi2-tx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car MIPI DSI/CSI-2 Encoder
> +
> +maintainers:
> +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> +
> +description: |
> +  This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> +  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> +  to four data lanes.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - renesas,r8a779a0-dsi-csi2-tx # for V3U

Only a potential nit ...

Is it worth moving the "# for V3U" over a bit to allow for extended
compatibles in the future without re-aligning the table?

Looks like 37 chars before it currently, it could at least move to
position 40.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Functional clock
> +      - description: DSI (and CSI-2) functional clock
> +      - description: PLL reference clock
> +
> +  clock-names:
> +    items:
> +      - const: fck
> +      - const: dsi
> +      - const: pll
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Parallel input port
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description: DSI/CSI-2 output port
> +
> +        properties:
> +          endpoint:
> +            $ref: /schemas/media/video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +            properties:
> +              data-lanes:
> +                minItems: 1
> +                maxItems: 4
> +
> +            required:
> +              - data-lanes
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +  - resets
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779a0-cpg-mssr.h>
> +    #include <dt-bindings/power/r8a779a0-sysc.h>
> +
> +    dsi0: dsi-encoder@fed80000 {
> +        compatible = "renesas,r8a779a0-dsi-csi2-tx";
> +        reg = <0xfed80000 0x10000>;
> +        power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>;
> +        clocks = <&cpg CPG_MOD 415>,
> +                 <&cpg CPG_CORE R8A779A0_CLK_DSI>,
> +                 <&cpg CPG_CORE R8A779A0_CLK_CP>;
> +        clock-names = "fck", "dsi", "pll";

is the CP/PLL clock actually needed?

I don't see any other gen3 peripheral referencing it.

Is it expected to be required for calculations in the DSI encoder?

Otherwise,

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>


> +        resets = <&cpg 415>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                dsi0_in: endpoint {
> +                    remote-endpoint = <&du_out_dsi0>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                dsi0_out: endpoint {
> +                    data-lanes = <1 2>;
> +                    remote-endpoint = <&sn65dsi86_in>;
> +                };
> +            };
> +        };
> +    };
> +...
>
Laurent Pinchart June 23, 2021, 1:06 p.m. UTC | #2 
On Wed, Jun 23, 2021 at 10:06:37AM +0100, Kieran Bingham wrote:
> Hi Laurent,
> 
> On 23/06/2021 04:46, Laurent Pinchart wrote:
> > The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
> > can operate in either DSI or CSI-2 mode, with up to four data lanes.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > new file mode 100644
> > index 000000000000..7e1b606a65ea
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > @@ -0,0 +1,118 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/renesas,dsi-csi2-tx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas R-Car MIPI DSI/CSI-2 Encoder
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > +
> > +description: |
> > +  This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> > +  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> > +  to four data lanes.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,r8a779a0-dsi-csi2-tx # for V3U
> 
> Only a potential nit ...
> 
> Is it worth moving the "# for V3U" over a bit to allow for extended
> compatibles in the future without re-aligning the table?
> 
> Looks like 37 chars before it currently, it could at least move to
> position 40.

If that's all it requires to make you happy, no problem :-)

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: Functional clock
> > +      - description: DSI (and CSI-2) functional clock
> > +      - description: PLL reference clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: fck
> > +      - const: dsi
> > +      - const: pll
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  resets:
> > +    maxItems: 1
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: Parallel input port
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/$defs/port-base
> > +        unevaluatedProperties: false
> > +        description: DSI/CSI-2 output port
> > +
> > +        properties:
> > +          endpoint:
> > +            $ref: /schemas/media/video-interfaces.yaml#
> > +            unevaluatedProperties: false
> > +
> > +            properties:
> > +              data-lanes:
> > +                minItems: 1
> > +                maxItems: 4
> > +
> > +            required:
> > +              - data-lanes
> > +
> > +    required:
> > +      - port@0
> > +      - port@1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - power-domains
> > +  - resets
> > +  - ports
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779a0-cpg-mssr.h>
> > +    #include <dt-bindings/power/r8a779a0-sysc.h>
> > +
> > +    dsi0: dsi-encoder@fed80000 {
> > +        compatible = "renesas,r8a779a0-dsi-csi2-tx";
> > +        reg = <0xfed80000 0x10000>;
> > +        power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>;
> > +        clocks = <&cpg CPG_MOD 415>,
> > +                 <&cpg CPG_CORE R8A779A0_CLK_DSI>,
> > +                 <&cpg CPG_CORE R8A779A0_CLK_CP>;
> > +        clock-names = "fck", "dsi", "pll";
> 
> is the CP/PLL clock actually needed?
> 
> I don't see any other gen3 peripheral referencing it.
> 
> Is it expected to be required for calculations in the DSI encoder?

It's listed in the datasheet as the DSI PLL input clock. The driver
still uses the old "extal" name though, which I'll fix.

> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> > +        resets = <&cpg 415>;
> > +
> > +        ports {
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            port@0 {
> > +                reg = <0>;
> > +                dsi0_in: endpoint {
> > +                    remote-endpoint = <&du_out_dsi0>;
> > +                };
> > +            };
> > +
> > +            port@1 {
> > +                reg = <1>;
> > +                dsi0_out: endpoint {
> > +                    data-lanes = <1 2>;
> > +                    remote-endpoint = <&sn65dsi86_in>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +...
> >
Geert Uytterhoeven June 23, 2021, 1:12 p.m. UTC | #3 
Hi Kieran,

On Wed, Jun 23, 2021 at 11:06 AM Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
> On 23/06/2021 04:46, Laurent Pinchart wrote:
> > The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
> > can operate in either DSI or CSI-2 mode, with up to four data lanes.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++++++++++++++++++
> >  1 file changed, 118 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > new file mode 100644
> > index 000000000000..7e1b606a65ea
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
> > @@ -0,0 +1,118 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/renesas,dsi-csi2-tx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas R-Car MIPI DSI/CSI-2 Encoder
> > +
> > +maintainers:
> > +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > +
> > +description: |
> > +  This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
> > +  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
> > +  to four data lanes.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,r8a779a0-dsi-csi2-tx # for V3U
>
> Only a potential nit ...
>
> Is it worth moving the "# for V3U" over a bit to allow for extended
> compatibles in the future without re-aligning the table?
>
> Looks like 37 chars before it currently, it could at least move to
> position 40.

Happy predicting the future ;-)

Did you take into account adding items and/or oneOf, which will
impact alignment, too?

Gr{oetje,eeting}s,

                        Geert
Kieran Bingham June 23, 2021, 2:36 p.m. UTC | #4 
On 23/06/2021 14:12, Geert Uytterhoeven wrote:
> Hi Kieran,
> 
> On Wed, Jun 23, 2021 at 11:06 AM Kieran Bingham
> <kieran.bingham@ideasonboard.com> wrote:
>> On 23/06/2021 04:46, Laurent Pinchart wrote:
>>> The R-Car MIPI DSI/CSI-2 TX is embedded in the Renesas R-Car V3U SoC. It
>>> can operate in either DSI or CSI-2 mode, with up to four data lanes.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> ---
>>>  .../display/bridge/renesas,dsi-csi2-tx.yaml   | 118 ++++++++++++++++++
>>>  1 file changed, 118 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>>> new file mode 100644
>>> index 000000000000..7e1b606a65ea
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
>>> @@ -0,0 +1,118 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/display/bridge/renesas,dsi-csi2-tx.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Renesas R-Car MIPI DSI/CSI-2 Encoder
>>> +
>>> +maintainers:
>>> +  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> +
>>> +description: |
>>> +  This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
>>> +  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
>>> +  to four data lanes.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - renesas,r8a779a0-dsi-csi2-tx # for V3U
>>
>> Only a potential nit ...
>>
>> Is it worth moving the "# for V3U" over a bit to allow for extended
>> compatibles in the future without re-aligning the table?
>>
>> Looks like 37 chars before it currently, it could at least move to
>> position 40.
> 
> Happy predicting the future ;-)
> 
> Did you take into account adding items and/or oneOf, which will
> impact alignment, too?

Maybe it should be way over at 60 chars then ;-)

Not a big issue, I don't mind either way, I'd just indent a little to
try to save when someone adds small updates that then requires a big
table change.


Ps. .. the lottery numbers this week are ...


> Gr{oetje,eeting}s,
> 
>                         Geert
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
new file mode 100644
index 000000000000..7e1b606a65ea
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi-csi2-tx.yaml
@@ -0,0 +1,118 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/renesas,dsi-csi2-tx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car MIPI DSI/CSI-2 Encoder
+
+maintainers:
+  - Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
+
+description: |
+  This binding describes the MIPI DSI/CSI-2 encoder embedded in the Renesas
+  R-Car V3U SoC. The encoder can operate in either DSI or CSI-2 mode, with up
+  to four data lanes.
+
+properties:
+  compatible:
+    enum:
+      - renesas,r8a779a0-dsi-csi2-tx # for V3U
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Functional clock
+      - description: DSI (and CSI-2) functional clock
+      - description: PLL reference clock
+
+  clock-names:
+    items:
+      - const: fck
+      - const: dsi
+      - const: pll
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Parallel input port
+
+      port@1:
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: DSI/CSI-2 output port
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              data-lanes:
+                minItems: 1
+                maxItems: 4
+
+            required:
+              - data-lanes
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+  - resets
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779a0-cpg-mssr.h>
+    #include <dt-bindings/power/r8a779a0-sysc.h>
+
+    dsi0: dsi-encoder@fed80000 {
+        compatible = "renesas,r8a779a0-dsi-csi2-tx";
+        reg = <0xfed80000 0x10000>;
+        power-domains = <&sysc R8A779A0_PD_ALWAYS_ON>;
+        clocks = <&cpg CPG_MOD 415>,
+                 <&cpg CPG_CORE R8A779A0_CLK_DSI>,
+                 <&cpg CPG_CORE R8A779A0_CLK_CP>;
+        clock-names = "fck", "dsi", "pll";
+        resets = <&cpg 415>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                dsi0_in: endpoint {
+                    remote-endpoint = <&du_out_dsi0>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                dsi0_out: endpoint {
+                    data-lanes = <1 2>;
+                    remote-endpoint = <&sn65dsi86_in>;
+                };
+            };
+        };
+    };
+...