[7/7] dt-bindings: display: Document Cadence MHDP HDMI/DP bindings

Commit Message

Sandor Yu June 1, 2020, 6:17 a.m. UTC

From: Sandor Yu <[e-mail]>

Document the bindings used for the Cadence MHDP HDMI/DP bridge.

Signed-off-by: Sandor Yu <[e-mail]>
---
 .../bindings/display/bridge/cdns,mhdp.yaml    | 46 +++++++++++++++
 .../devicetree/bindings/display/imx/mhdp.yaml | 59 +++++++++++++++++++
 2 files changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
 create mode 100644 Documentation/devicetree/bindings/display/imx/mhdp.yaml

Comments

Laurent Pinchart June 2, 2020, 11:44 p.m. UTC | #1

Hi Sandor,

Thank you for the patch.

On Mon, Jun 01, 2020 at 02:17:37PM +0800, [e-mail] wrote:
> From: Sandor Yu <[e-mail]>
> 
> Document the bindings used for the Cadence MHDP HDMI/DP bridge.
> 
> Signed-off-by: Sandor Yu <[e-mail]>
> ---
>  .../bindings/display/bridge/cdns,mhdp.yaml    | 46 +++++++++++++++
>  .../devicetree/bindings/display/imx/mhdp.yaml | 59 +++++++++++++++++++

Please split the patch in two.

>  2 files changed, 105 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
>  create mode 100644 Documentation/devicetree/bindings/display/imx/mhdp.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> new file mode 100644
> index 000000000000..aa23feba744a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause))
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence MHDP TX Encoder
> +
> +maintainers:
> +  - Sandor Yu <[e-mail]>
> +
> +description: |
> +  Cadence MHDP Controller supports one or more of the protocols,
> +  such as HDMI and DisplayPort.
> +  Each protocol requires a different FW binaries.
> +
> +  This document defines device tree properties for the Cadence MHDP Encoder
> +  (CDNS MHDP TX). It doesn't constitue a device tree binding
> +  specification by itself but is meant to be referenced by platform-specific
> +  device tree bindings.
> +
> +  When referenced from platform device tree bindings the properties defined in
> +  this document are defined as follows. The platform device tree bindings are
> +  responsible for defining whether each property is required or optional.
> +
> +properties:
> +  reg:
> +    maxItems: 1
> +    description: Memory mapped base address and length of the MHDP TX registers.
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:
> +    - const: plug_in
> +      description: Hotplug detect interrupter for cable plugin event.
> +    - const: plug_out
> +      description: Hotplug detect interrupter for cable plugout event.

Does the IP core really have two different interrupt lines, one for
hot-plug and one for hot-unplug ? That's a very unusual design.

> +
> +  port:
> +    type: object
> +    description: |
> +      The connectivity of the MHDP TX with the rest of the system is
> +      expressed in using ports as specified in the device graph bindings defined
> +      in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
> +      is platform-specific.
> diff --git a/Documentation/devicetree/bindings/display/imx/mhdp.yaml b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> new file mode 100644
> index 000000000000..17850cfd1cb1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/mhdp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cadence MHDP Encoder
> +
> +maintainers:
> +  - Sandor Yu <[e-mail]>
> +
> +description: |
> +  The MHDP transmitter is a Cadence HD Display TX controller IP
> +  with a companion PHY IP.
> +  The MHDP supports one or more of the protocols,
> +  such as HDMI(1.4 & 2.0), DisplayPort(1.2).
> +  switching between the two modes (HDMI and DisplayPort)
> +  requires reloading the appropriate FW

Does the IP core integrated in the imx8mp SoCs (as that is what this
binding targets) support both HDMI and DP ? If not this should be
reworded to be more specific to the SoC.

> +
> +  These DT bindings follow the Cadence MHDP TX bindings defined in
> +  Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml with the
> +  following device-specific properties.
> +
> +Properties:

Have you tried validating this with make dt_binding_check ? See
Documentation/devicetree/writing-schema.rst for more information.

> +  compatible:
> +    enum:
> +      - nxp,imx8mq-cdns-hdmi
> +      - nxp,imx8mq-cdns-dp
> +
> +  reg: See cdns,mhdp.yaml.

This isn't how bindings are referenced. You need to reference the parent
binding with $ref, either globally, or on an individual property basis.

> +
> +  interrupts: See cdns,mhdp.yaml.
> +
> +  interrupt-names: See cdns,mhdp.yaml.

That's it ? No clocks, no power domains, no resets, no PHYs (especially
given that you mention a PHY companion IP above) ?

> +
> +  ports: See cdns,mhdp.yaml.

This isn't correct. Please soo of-graph.txt. If can have either one port
node, or one ports node that contains one of more port subnodes. In this
case you need at least two ports, one for the input to the HDMI encoder,
and one for the HDMI output. The latter should be connected to a DT node
representing the HDMI connector. Yuo can search for "hdmi-connector" in
the .dts files in the kernel for plenty of examples.

> +
> +Required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - ports
> +
> +Example:
> +  - |
> +    mhdp: mhdp@32c00000 {
> +      compatible = "nxp,imx8mq-cdns-hdmi";
> +      reg = <0x32c00000 0x100000>;
> +      interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> +                   <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> +      interrupt-names = "plug_in", "plug_out";
> +
> +      ports {
> +        mhdp_in: endpoint {
> +          remote-endpoint = <&dcss_out>;
> +        };
> +      };
> +    };

Sandor Yu June 4, 2020, 9:32 a.m. UTC | #2

Hi Laurent,

Thanks your review comments.

> -----Original Message-----
> From: Laurent Pinchart <[e-mail]>
> Sent: Wednesday, June 3, 2020 7:44 AM
> To: Sandor Yu <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; dl-linux-imx <[e-mail]>
> Subject: [EXT] Re: [PATCH 7/7] dt-bindings: display: Document Cadence MHDP
> HDMI/DP bindings
> 
> Caution: EXT Email
> 
> Hi Sandor,
> 
> Thank you for the patch.
> 
> On Mon, Jun 01, 2020 at 02:17:37PM +0800, [e-mail] wrote:
> > From: Sandor Yu <[e-mail]>
> >
> > Document the bindings used for the Cadence MHDP HDMI/DP bridge.
> >
> > Signed-off-by: Sandor Yu <[e-mail]>
> > ---
> >  .../bindings/display/bridge/cdns,mhdp.yaml    | 46 +++++++++++++++
> >  .../devicetree/bindings/display/imx/mhdp.yaml | 59
> > +++++++++++++++++++
> 
> Please split the patch in two.
OK, I will split it later.
> 
> >  2 files changed, 105 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> >  create mode 100644
> > Documentation/devicetree/bindings/display/imx/mhdp.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> > new file mode 100644
> > index 000000000000..aa23feba744a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fcdns%2Cmhdp.yaml%23&a
> mp;dat
> >
> +a=02%7C01%7Csandor.yu%40nxp.com%7C7862af2fa05b490b6fe908d8074eea
> 14%7C
> >
> +686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637267382802978543&
> amp;sda
> >
> +ta=1AoEDe9F2v7fy0RRWkn%2BgEXWgvt78E1D8DqmCf8AsnU%3D&amp;reser
> ved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Csand
> or.yu
> >
> +%40nxp.com%7C7862af2fa05b490b6fe908d8074eea14%7C686ea1d3bc2b4c6f
> a92cd
> >
> +99c5c301635%7C0%7C0%7C637267382802988539&amp;sdata=X5HtTsR2roO
> %2FyqOI
> > +JbaLy3hEcxjmU5QL5b1UHXOdbLg%3D&amp;reserved=0
> > +
> > +title: Cadence MHDP TX Encoder
> > +
> > +maintainers:
> > +  - Sandor Yu <[e-mail]>
> > +
> > +description: |
> > +  Cadence MHDP Controller supports one or more of the protocols,
> > +  such as HDMI and DisplayPort.
> > +  Each protocol requires a different FW binaries.
> > +
> > +  This document defines device tree properties for the Cadence MHDP
> > + Encoder  (CDNS MHDP TX). It doesn't constitue a device tree binding
> > + specification by itself but is meant to be referenced by
> > + platform-specific  device tree bindings.
> > +
> > +  When referenced from platform device tree bindings the properties
> > + defined in  this document are defined as follows. The platform
> > + device tree bindings are  responsible for defining whether each property
> is required or optional.
> > +
> > +properties:
> > +  reg:
> > +    maxItems: 1
> > +    description: Memory mapped base address and length of the MHDP TX
> registers.
> > +
> > +  interrupts:
> > +    maxItems: 2
> > +
> > +  interrupt-names:
> > +    - const: plug_in
> > +      description: Hotplug detect interrupter for cable plugin event.
> > +    - const: plug_out
> > +      description: Hotplug detect interrupter for cable plugout event.
> 
> Does the IP core really have two different interrupt lines, one for hot-plug and
> one for hot-unplug ? That's a very unusual design.
> 
These two interrupter generated by SOC.
The IP Core haven't provide interrupt lines to SOC, it only provide HPD status in Firmware.
  
> > +
> > +  port:
> > +    type: object
> > +    description: |
> > +      The connectivity of the MHDP TX with the rest of the system is
> > +      expressed in using ports as specified in the device graph bindings
> defined
> > +      in Documentation/devicetree/bindings/graph.txt. The numbering of
> the ports
> > +      is platform-specific.
> > diff --git a/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> > b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> > new file mode 100644
> > index 000000000000..17850cfd1cb1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fmhdp.yaml%23&amp;data=0
> 2%7C
> >
> +01%7Csandor.yu%40nxp.com%7C7862af2fa05b490b6fe908d8074eea14%7C68
> 6ea1d
> >
> +3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637267382802988539&amp;sdat
> a=88VM
> > +M6TP6Sd%2FwGnKsZ7REO1S5FNY5UJ6ll0H5pecNrY%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Csand
> or.yu
> >
> +%40nxp.com%7C7862af2fa05b490b6fe908d8074eea14%7C686ea1d3bc2b4c6f
> a92cd
> >
> +99c5c301635%7C0%7C0%7C637267382802988539&amp;sdata=X5HtTsR2roO
> %2FyqOI
> > +JbaLy3hEcxjmU5QL5b1UHXOdbLg%3D&amp;reserved=0
> > +
> > +title: Cadence MHDP Encoder
> > +
> > +maintainers:
> > +  - Sandor Yu <[e-mail]>
> > +
> > +description: |
> > +  The MHDP transmitter is a Cadence HD Display TX controller IP
> > +  with a companion PHY IP.
> > +  The MHDP supports one or more of the protocols,
> > +  such as HDMI(1.4 & 2.0), DisplayPort(1.2).
> > +  switching between the two modes (HDMI and DisplayPort)
> > +  requires reloading the appropriate FW
> 
> Does the IP core integrated in the imx8mp SoCs (as that is what this binding
> targets) support both HDMI and DP ? If not this should be reworded to be
> more specific to the SoC.
> 
Yes, the IP core have one MCU, it could run different firmware to support HDMI or DP. 
> > +
> > +  These DT bindings follow the Cadence MHDP TX bindings defined in
> > + Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml with
> > + the  following device-specific properties.
> > +
> > +Properties:
> 
> Have you tried validating this with make dt_binding_check ? See
> Documentation/devicetree/writing-schema.rst for more information.
> 
I have run the online yaml checker, I will check it with make dt_binding_check, thanks.
> > +  compatible:
> > +    enum:
> > +      - nxp,imx8mq-cdns-hdmi
> > +      - nxp,imx8mq-cdns-dp
> > +
> > +  reg: See cdns,mhdp.yaml.
> 
> This isn't how bindings are referenced. You need to reference the parent
> binding with $ref, either globally, or on an individual property basis.
OK
> 
> > +
> > +  interrupts: See cdns,mhdp.yaml.
> > +
> > +  interrupt-names: See cdns,mhdp.yaml.
> 
> That's it ? No clocks, no power domains, no resets, no PHYs (especially given
> that you mention a PHY companion IP above) ?
> 
Yes, iMX8MQ HDMI/DP firmware loaded by ROM code for security check,
IP core clock and power will management by ROM code, it will keep in ON status when device bootup.
The PHY configurated in platform files, it will generate pixel clock for HDMI controller. 
So no clocks, power, reset and PHY.
> > +
> > +  ports: See cdns,mhdp.yaml.
> 
> This isn't correct. Please soo of-graph.txt. If can have either one port node, or
> one ports node that contains one of more port subnodes. In this case you
> need at least two ports, one for the input to the HDMI encoder, and one for
> the HDMI output. The latter should be connected to a DT node representing
> the HDMI connector. Yuo can search for "hdmi-connector" in the .dts files in
> the kernel for plenty of examples.
OK, I will add more information later.
> 
> > +
> > +Required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - interrupt-names
> > +  - ports
> > +
> > +Example:
> > +  - |
> > +    mhdp: mhdp@32c00000 {
> > +      compatible = "nxp,imx8mq-cdns-hdmi";
> > +      reg = <0x32c00000 0x100000>;
> > +      interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
> > +                   <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
> > +      interrupt-names = "plug_in", "plug_out";
> > +
> > +      ports {
> > +        mhdp_in: endpoint {
> > +          remote-endpoint = <&dcss_out>;
> > +        };
> > +      };
> > +    };
> 
> --
> Regards,
> 
> Laurent Pinchart

Regards,
Sandor

Patch

diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
new file mode 100644
index 000000000000..aa23feba744a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml
@@ -0,0 +1,46 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause))
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence MHDP TX Encoder
+
+maintainers:
+  - Sandor Yu <[e-mail]>
+
+description: |
+  Cadence MHDP Controller supports one or more of the protocols,
+  such as HDMI and DisplayPort.
+  Each protocol requires a different FW binaries.
+
+  This document defines device tree properties for the Cadence MHDP Encoder
+  (CDNS MHDP TX). It doesn't constitue a device tree binding
+  specification by itself but is meant to be referenced by platform-specific
+  device tree bindings.
+
+  When referenced from platform device tree bindings the properties defined in
+  this document are defined as follows. The platform device tree bindings are
+  responsible for defining whether each property is required or optional.
+
+properties:
+  reg:
+    maxItems: 1
+    description: Memory mapped base address and length of the MHDP TX registers.
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    - const: plug_in
+      description: Hotplug detect interrupter for cable plugin event.
+    - const: plug_out
+      description: Hotplug detect interrupter for cable plugout event.
+
+  port:
+    type: object
+    description: |
+      The connectivity of the MHDP TX with the rest of the system is
+      expressed in using ports as specified in the device graph bindings defined
+      in Documentation/devicetree/bindings/graph.txt. The numbering of the ports
+      is platform-specific.
diff --git a/Documentation/devicetree/bindings/display/imx/mhdp.yaml b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
new file mode 100644
index 000000000000..17850cfd1cb1
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/mhdp.yaml
@@ -0,0 +1,59 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/mhdp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cadence MHDP Encoder
+
+maintainers:
+  - Sandor Yu <[e-mail]>
+
+description: |
+  The MHDP transmitter is a Cadence HD Display TX controller IP
+  with a companion PHY IP.
+  The MHDP supports one or more of the protocols,
+  such as HDMI(1.4 & 2.0), DisplayPort(1.2).
+  switching between the two modes (HDMI and DisplayPort)
+  requires reloading the appropriate FW
+
+  These DT bindings follow the Cadence MHDP TX bindings defined in
+  Documentation/devicetree/bindings/display/bridge/cdns,mhdp.yaml with the
+  following device-specific properties.
+
+Properties:
+  compatible:
+    enum:
+      - nxp,imx8mq-cdns-hdmi
+      - nxp,imx8mq-cdns-dp
+
+  reg: See cdns,mhdp.yaml.
+
+  interrupts: See cdns,mhdp.yaml.
+
+  interrupt-names: See cdns,mhdp.yaml.
+
+  ports: See cdns,mhdp.yaml.
+
+Required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - ports
+
+Example:
+  - |
+    mhdp: mhdp@32c00000 {
+      compatible = "nxp,imx8mq-cdns-hdmi";
+      reg = <0x32c00000 0x100000>;
+      interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
+                   <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-names = "plug_in", "plug_out";
+
+      ports {
+        mhdp_in: endpoint {
+          remote-endpoint = <&dcss_out>;
+        };
+      };
+    };