[v3,104/105] dt-bindings: display: vc4: hdmi: Add BCM2711 HDMI controllers bindings

Commit Message

Maxime Ripard May 27, 2020, 3:49 p.m. UTC

The HDMI controllers found in the BCM2711 SoC need some adjustments to the
bindings, especially since the registers have been shuffled around in more
register ranges.

Cc: Rob Herring <[e-mail]>
Cc: [e-mail]
Signed-off-by: Maxime Ripard <[e-mail]>
---
 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml

Comments

Rob Herring May 29, 2020, 6:18 p.m. UTC | #1

On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> bindings, especially since the registers have been shuffled around in more
> register ranges.
> 
> Cc: Rob Herring <[e-mail]>
> Cc: [e-mail]
> Signed-off-by: Maxime Ripard <[e-mail]>
> ---
>  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> new file mode 100644
> index 000000000000..6091fe3d315b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license...

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> +
> +maintainers:
> +  - Eric Anholt <[e-mail]>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm2711-hdmi0
> +      - brcm,bcm2711-hdmi1

What's the difference between the 2 blocks? 

> +
> +  reg:
> +    items:
> +      - description: HDMI controller register range
> +      - description: DVP register range
> +      - description: HDMI PHY register range
> +      - description: Rate Manager register range
> +      - description: Packet RAM register range
> +      - description: Metadata RAM register range
> +      - description: CSC register range
> +      - description: CEC register range
> +      - description: HD register range
> +
> +  reg-names:
> +    items:
> +      - const: hdmi
> +      - const: dvp
> +      - const: phy
> +      - const: rm
> +      - const: packet
> +      - const: metadata
> +      - const: csc
> +      - const: cec
> +      - const: hd
> +
> +  clocks:
> +    description: The HDMI state machine clock
> +
> +  clock-names:
> +    const: hdmi
> +
> +  ddc:
> +    allOf:
> +      - $ref: /schemas/types.yaml#/definitions/phandle
> +    description: >
> +      Phandle of the I2C controller used for DDC EDID probing

Goes in the connector.

And isn't the standard name ddc-i2c-bus?

> +
> +  hpd-gpios:
> +    description: >
> +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> +      as an interrupt/status bit in the HDMI controller itself)

Goes in the connector.

> +
> +  dmas:
> +    maxItems: 1
> +    description: >
> +      Should contain one entry pointing to the DMA channel used to
> +      transfer audio data.
> +
> +  dma-names:
> +    const: audio-rx
> +
> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - resets
> +  - ddc
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    hdmi0: hdmi@7ef00700 {
> +        compatible = "brcm,bcm2711-hdmi0";
> +        reg = <0x7ef00700 0x300>,
> +              <0x7ef00300 0x200>,
> +              <0x7ef00f00 0x80>,
> +              <0x7ef00f80 0x80>,
> +              <0x7ef01b00 0x200>,
> +              <0x7ef01f00 0x400>,
> +              <0x7ef00200 0x80>,
> +              <0x7ef04300 0x100>,
> +              <0x7ef20000 0x100>;
> +        reg-names = "hdmi",
> +                    "dvp",
> +                    "phy",
> +                    "rm",
> +                    "packet",
> +                    "metadata",
> +                    "csc",
> +                    "cec",
> +                    "hd";
> +        clocks = <&firmware_clocks 13>;
> +        clock-names = "hdmi";
> +        resets = <&dvp 0>;
> +        ddc = <&ddc0>;
> +    };
> +
> +...
> -- 
> git-series 0.9.1

Maxime Ripard June 2, 2020, 3:08 p.m. UTC | #2

Hi Rob,

On Fri, May 29, 2020 at 12:18:33PM -0600, Rob Herring wrote:
> On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> > The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> > bindings, especially since the registers have been shuffled around in more
> > register ranges.
> > 
> > Cc: Rob Herring <[e-mail]>
> > Cc: [e-mail]
> > Signed-off-by: Maxime Ripard <[e-mail]>
> > ---
> >  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 109 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > new file mode 100644
> > index 000000000000..6091fe3d315b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > @@ -0,0 +1,109 @@
> > +# SPDX-License-Identifier: GPL-2.0
> 
> Dual license...
> 
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Eric Anholt <[e-mail]>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - brcm,bcm2711-hdmi0
> > +      - brcm,bcm2711-hdmi1
> 
> What's the difference between the 2 blocks? 

The register layout and the lane mapping in the PHY change a bit.

> > +
> > +  reg:
> > +    items:
> > +      - description: HDMI controller register range
> > +      - description: DVP register range
> > +      - description: HDMI PHY register range
> > +      - description: Rate Manager register range
> > +      - description: Packet RAM register range
> > +      - description: Metadata RAM register range
> > +      - description: CSC register range
> > +      - description: CEC register range
> > +      - description: HD register range
> > +
> > +  reg-names:
> > +    items:
> > +      - const: hdmi
> > +      - const: dvp
> > +      - const: phy
> > +      - const: rm
> > +      - const: packet
> > +      - const: metadata
> > +      - const: csc
> > +      - const: cec
> > +      - const: hd
> > +
> > +  clocks:
> > +    description: The HDMI state machine clock
> > +
> > +  clock-names:
> > +    const: hdmi
> > +
> > +  ddc:
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/phandle
> > +    description: >
> > +      Phandle of the I2C controller used for DDC EDID probing
> 
> Goes in the connector.
> 
> And isn't the standard name ddc-i2c-bus?
> 
> > +
> > +  hpd-gpios:
> > +    description: >
> > +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> > +      as an interrupt/status bit in the HDMI controller itself)
> 
> Goes in the connector.

If this was an entirely new binding, I would agree, but this is not
really the case here.

We discussed it already for the v2, and this binding is essentially the
same one than the bcm2835 HDMI controller.

I initially sent a patch adding conditionnals for the clocks and regs
differences too, and you asked to split the binding into a separate file
to simplify it a bit.

Supporting both the old binding, and the new one based on the connector
is going to make the code significantly more complicated, and I'm not
really sure why we would here.

Thanks!
Maxime

Rob Herring June 8, 2020, 11:01 p.m. UTC | #3

On Tue, Jun 2, 2020 at 9:08 AM Maxime Ripard <[e-mail]> wrote:
>
> Hi Rob,
>
> On Fri, May 29, 2020 at 12:18:33PM -0600, Rob Herring wrote:
> > On Wed, May 27, 2020 at 05:49:14PM +0200, Maxime Ripard wrote:
> > > The HDMI controllers found in the BCM2711 SoC need some adjustments to the
> > > bindings, especially since the registers have been shuffled around in more
> > > register ranges.
> > >
> > > Cc: Rob Herring <[e-mail]>
> > > Cc: [e-mail]
> > > Signed-off-by: Maxime Ripard <[e-mail]>
> > > ---
> > >  Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 109 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > > new file mode 100644
> > > index 000000000000..6091fe3d315b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
> > > @@ -0,0 +1,109 @@
> > > +# SPDX-License-Identifier: GPL-2.0
> >
> > Dual license...
> >
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Eric Anholt <[e-mail]>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - brcm,bcm2711-hdmi0
> > > +      - brcm,bcm2711-hdmi1
> >
> > What's the difference between the 2 blocks?
>
> The register layout and the lane mapping in the PHY change a bit.
>
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: HDMI controller register range
> > > +      - description: DVP register range
> > > +      - description: HDMI PHY register range
> > > +      - description: Rate Manager register range
> > > +      - description: Packet RAM register range
> > > +      - description: Metadata RAM register range
> > > +      - description: CSC register range
> > > +      - description: CEC register range
> > > +      - description: HD register range
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: hdmi
> > > +      - const: dvp
> > > +      - const: phy
> > > +      - const: rm
> > > +      - const: packet
> > > +      - const: metadata
> > > +      - const: csc
> > > +      - const: cec
> > > +      - const: hd
> > > +
> > > +  clocks:
> > > +    description: The HDMI state machine clock
> > > +
> > > +  clock-names:
> > > +    const: hdmi
> > > +
> > > +  ddc:
> > > +    allOf:
> > > +      - $ref: /schemas/types.yaml#/definitions/phandle
> > > +    description: >
> > > +      Phandle of the I2C controller used for DDC EDID probing
> >
> > Goes in the connector.
> >
> > And isn't the standard name ddc-i2c-bus?
> >
> > > +
> > > +  hpd-gpios:
> > > +    description: >
> > > +      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
> > > +      as an interrupt/status bit in the HDMI controller itself)
> >
> > Goes in the connector.
>
> If this was an entirely new binding, I would agree, but this is not
> really the case here.
>
> We discussed it already for the v2, and this binding is essentially the
> same one than the bcm2835 HDMI controller.
>
> I initially sent a patch adding conditionnals for the clocks and regs
> differences too, and you asked to split the binding into a separate file
> to simplify it a bit.

Ah... Best to assume I don't remember what happened last week. ;)

> Supporting both the old binding, and the new one based on the connector
> is going to make the code significantly more complicated, and I'm not
> really sure why we would here.

I'm not really sure I agree on the complexity, but that can be a
discussion for another day.

Reviewed-by: Rob Herring <[e-mail]>

Rob

Patch

diff --git a/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
new file mode 100644
index 000000000000..6091fe3d315b
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/brcm,bcm2711-hdmi.yaml
@@ -0,0 +1,109 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/brcm,bcm2711-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Broadcom BCM2711 HDMI Controller Device Tree Bindings
+
+maintainers:
+  - Eric Anholt <[e-mail]>
+
+properties:
+  compatible:
+    enum:
+      - brcm,bcm2711-hdmi0
+      - brcm,bcm2711-hdmi1
+
+  reg:
+    items:
+      - description: HDMI controller register range
+      - description: DVP register range
+      - description: HDMI PHY register range
+      - description: Rate Manager register range
+      - description: Packet RAM register range
+      - description: Metadata RAM register range
+      - description: CSC register range
+      - description: CEC register range
+      - description: HD register range
+
+  reg-names:
+    items:
+      - const: hdmi
+      - const: dvp
+      - const: phy
+      - const: rm
+      - const: packet
+      - const: metadata
+      - const: csc
+      - const: cec
+      - const: hd
+
+  clocks:
+    description: The HDMI state machine clock
+
+  clock-names:
+    const: hdmi
+
+  ddc:
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/phandle
+    description: >
+      Phandle of the I2C controller used for DDC EDID probing
+
+  hpd-gpios:
+    description: >
+      The GPIO pin for the HDMI hotplug detect (if it doesn't appear
+      as an interrupt/status bit in the HDMI controller itself)
+
+  dmas:
+    maxItems: 1
+    description: >
+      Should contain one entry pointing to the DMA channel used to
+      transfer audio data.
+
+  dma-names:
+    const: audio-rx
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - clocks
+  - resets
+  - ddc
+
+additionalProperties: false
+
+examples:
+  - |
+    hdmi0: hdmi@7ef00700 {
+        compatible = "brcm,bcm2711-hdmi0";
+        reg = <0x7ef00700 0x300>,
+              <0x7ef00300 0x200>,
+              <0x7ef00f00 0x80>,
+              <0x7ef00f80 0x80>,
+              <0x7ef01b00 0x200>,
+              <0x7ef01f00 0x400>,
+              <0x7ef00200 0x80>,
+              <0x7ef04300 0x100>,
+              <0x7ef20000 0x100>;
+        reg-names = "hdmi",
+                    "dvp",
+                    "phy",
+                    "rm",
+                    "packet",
+                    "metadata",
+                    "csc",
+                    "cec",
+                    "hd";
+        clocks = <&firmware_clocks 13>;
+        clock-names = "hdmi";
+        resets = <&dvp 0>;
+        ddc = <&ddc0>;
+    };
+
+...