diff mbox series

[8/9] media: dt-bindings: Add Intel Displayport RX IP

Message ID 20240212131323.2162161-9-panikiel@google.com
State Changes Requested
Headers show
Series Add Chameleon v3 video support | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dt-meta-schema fail build log

Commit Message

Paweł Anikiel Feb. 12, 2024, 1:13 p.m. UTC
The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
capture and Multi-Stream Transport. The user guide can be found here:

https://www.intel.com/programmable/technical-pdfs/683273.pdf

Signed-off-by: Paweł Anikiel <panikiel@google.com>
---
 .../devicetree/bindings/media/intel,dprx.yaml | 125 ++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml

Comments

Rob Herring (Arm) Feb. 12, 2024, 2:35 p.m. UTC | #1
On Mon, 12 Feb 2024 13:13:22 +0000, Paweł Anikiel wrote:
> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> capture and Multi-Stream Transport. The user guide can be found here:
> 
> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> 
> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> ---
>  .../devicetree/bindings/media/intel,dprx.yaml | 125 ++++++++++++++++++
>  1 file changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/media/intel,dprx.example.dts:28.27-28 syntax error
make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/media/intel,dprx.example.dtb] Error 1

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240212131323.2162161-9-panikiel@google.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Paweł Anikiel Feb. 12, 2024, 3:52 p.m. UTC | #2
On Mon, Feb 12, 2024 at 3:35 PM Rob Herring <robh@kernel.org> wrote:
>
>
> On Mon, 12 Feb 2024 13:13:22 +0000, Paweł Anikiel wrote:
> > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > capture and Multi-Stream Transport. The user guide can be found here:
> >
> > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> >
> > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > ---
> >  .../devicetree/bindings/media/intel,dprx.yaml | 125 ++++++++++++++++++
> >  1 file changed, 125 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/media/intel,dprx.example.dts:28.27-28 syntax error
> make[2]: *** [scripts/Makefile.lib:419: Documentation/devicetree/bindings/media/intel,dprx.example.dtb] Error 1
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240212131323.2162161-9-panikiel@google.com
>
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
>

As with the previous patch, I was missing a #include in the example. I
will include the fix in v2.
Conor Dooley Feb. 15, 2024, 5:26 p.m. UTC | #3
Yo,

On Mon, Feb 12, 2024 at 01:13:22PM +0000, Paweł Anikiel wrote:
> The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> capture and Multi-Stream Transport. The user guide can be found here:
> 
> https://www.intel.com/programmable/technical-pdfs/683273.pdf
> 
> Signed-off-by: Paweł Anikiel <panikiel@google.com>
> ---
>  .../devicetree/bindings/media/intel,dprx.yaml | 125 ++++++++++++++++++
>  1 file changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> new file mode 100644
> index 000000000000..3ed37e0a4a94
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Intel DisplayPort RX IP
> +
> +maintainers:
> +  - Paweł Anikiel <panikiel@google.com>
> +
> +properties:
> +  compatible:
> +    const: intel,dprx

Please version this compatible, given that is it for an FPGA IP.
I could not find an example of another intel IP that had versioning, but
there's plenty of xilinx stuff you can get inspiration from.

> +  reg:
> +    items:
> +      - description: core registers
> +      - description: irq registers
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  intel,has-mst:

Mostly this looks fine, but this property drew my eye.
Firstly, I'd probably call this "intel,multi-stream-support" rather than
"intel,has-mst".

> +    type: boolean
> +    description: The device supports Multi-Stream Transport

Secondly, there are many many configuration parameters for this IP,
but you have chosen to document just one.
Are all other configuration parameters currently in their default
states or ignored by the driver? If not, please at least document all
configuration settings that you rely on - for example the max stream
count or audio packet encoding.

> +
> +  port:
> +    $ref: /schemas/graph.yaml#/properties/port
> +    description: SST main link
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 0 or SST main link
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 1
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 2
> +
> +      port@3:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: MST virtual channel 3
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +allOf:
> +  - if:
> +      required:
> +        - intel,has-mst
> +    then:
> +      required:
> +        - ports
> +    else:
> +      required:
> +        - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dprx@c0062000 {

"dprx" isn't a class of device, please try to use a generic node name
here.

Thanks,
Conor.

> +        compatible = "intel,dprx";
> +        reg = <0xc0062000 0x800>,
> +              <0xc0060f80 0x10>;
> +        interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +        intel,has-mst;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                dprx_mst_0: endpoint {
> +                    remote-endpoint = <&fb_mst0_0>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                dprx_mst_1: endpoint {
> +                    remote-endpoint = <&fb_mst1_0>;
> +                };
> +            };
> +
> +            port@2 {
> +                reg = <2>;
> +                dprx_mst_2: endpoint {
> +                    remote-endpoint = <&fb_mst2_0>;
> +                };
> +            };
> +
> +            port@3 {
> +                reg = <3>;
> +                dprx_mst_3: endpoint {
> +                    remote-endpoint = <&fb_mst3_0>;
> +                };
> +            };
> +        };
> +    };
> +
> +  - |
> +    dprx@c0064000 {
> +        compatible = "intel,dprx";
> +        reg = <0xc0064000 0x800>,
> +              <0xc0060fe0 0x10>;
> +        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
> +
> +        port {
> +            dprx_sst_0: endpoint {
> +                remote-endpoint = <&fb_sst_0>;
> +            };
> +        };
> +    };
> -- 
> 2.43.0.687.g38aa6559b0-goog
>
Paweł Anikiel Feb. 21, 2024, 4:30 p.m. UTC | #4
On Thu, Feb 15, 2024 at 6:26 PM Conor Dooley <conor@kernel.org> wrote:
>
> Yo,
>
> On Mon, Feb 12, 2024 at 01:13:22PM +0000, Paweł Anikiel wrote:
> > The Intel Displayport RX IP is a part of the DisplayPort Intel FPGA IP
> > Core. It implements a DisplayPort 1.4 receiver capable of HBR3 video
> > capture and Multi-Stream Transport. The user guide can be found here:
> >
> > https://www.intel.com/programmable/technical-pdfs/683273.pdf
> >
> > Signed-off-by: Paweł Anikiel <panikiel@google.com>
> > ---
> >  .../devicetree/bindings/media/intel,dprx.yaml | 125 ++++++++++++++++++
> >  1 file changed, 125 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/intel,dprx.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > new file mode 100644
> > index 000000000000..3ed37e0a4a94
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
> > @@ -0,0 +1,125 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Intel DisplayPort RX IP
> > +
> > +maintainers:
> > +  - Paweł Anikiel <panikiel@google.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: intel,dprx
>
> Please version this compatible, given that is it for an FPGA IP.
> I could not find an example of another intel IP that had versioning, but
> there's plenty of xilinx stuff you can get inspiration from.

The IP has had a few different revisions, so I decided to just use the
"IP version" which is 20.0.1 for the version this driver is targeting.

>
> > +  reg:
> > +    items:
> > +      - description: core registers
> > +      - description: irq registers
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  intel,has-mst:
>
> Mostly this looks fine, but this property drew my eye.
> Firstly, I'd probably call this "intel,multi-stream-support" rather than
> "intel,has-mst".

Sure,

>
> > +    type: boolean
> > +    description: The device supports Multi-Stream Transport
>
> Secondly, there are many many configuration parameters for this IP,
> but you have chosen to document just one.
> Are all other configuration parameters currently in their default
> states or ignored by the driver? If not, please at least document all
> configuration settings that you rely on - for example the max stream
> count or audio packet encoding.

I looked at all the parameters listed in the user guide, and most of
them don't affect the driver. I listed the ones which are required,
and added support for the remaining ones in v2.

>
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    description: SST main link
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    properties:
> > +      port@0:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: MST virtual channel 0 or SST main link
> > +
> > +      port@1:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: MST virtual channel 1
> > +
> > +      port@2:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: MST virtual channel 2
> > +
> > +      port@3:
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +        description: MST virtual channel 3
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +
> > +allOf:
> > +  - if:
> > +      required:
> > +        - intel,has-mst
> > +    then:
> > +      required:
> > +        - ports
> > +    else:
> > +      required:
> > +        - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    dprx@c0062000 {
>
> "dprx" isn't a class of device, please try to use a generic node name
> here.

I couldn't find anything in the DT spec or any other similar examples,
so I chose dp-receiver.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/intel,dprx.yaml b/Documentation/devicetree/bindings/media/intel,dprx.yaml
new file mode 100644
index 000000000000..3ed37e0a4a94
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/intel,dprx.yaml
@@ -0,0 +1,125 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/intel,dprx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Intel DisplayPort RX IP
+
+maintainers:
+  - Paweł Anikiel <panikiel@google.com>
+
+properties:
+  compatible:
+    const: intel,dprx
+
+  reg:
+    items:
+      - description: core registers
+      - description: irq registers
+
+  interrupts:
+    maxItems: 1
+
+  intel,has-mst:
+    type: boolean
+    description: The device supports Multi-Stream Transport
+
+  port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description: SST main link
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: MST virtual channel 0 or SST main link
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: MST virtual channel 1
+
+      port@2:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: MST virtual channel 2
+
+      port@3:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: MST virtual channel 3
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - if:
+      required:
+        - intel,has-mst
+    then:
+      required:
+        - ports
+    else:
+      required:
+        - port
+
+additionalProperties: false
+
+examples:
+  - |
+    dprx@c0062000 {
+        compatible = "intel,dprx";
+        reg = <0xc0062000 0x800>,
+              <0xc0060f80 0x10>;
+        interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+        intel,has-mst;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                dprx_mst_0: endpoint {
+                    remote-endpoint = <&fb_mst0_0>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                dprx_mst_1: endpoint {
+                    remote-endpoint = <&fb_mst1_0>;
+                };
+            };
+
+            port@2 {
+                reg = <2>;
+                dprx_mst_2: endpoint {
+                    remote-endpoint = <&fb_mst2_0>;
+                };
+            };
+
+            port@3 {
+                reg = <3>;
+                dprx_mst_3: endpoint {
+                    remote-endpoint = <&fb_mst3_0>;
+                };
+            };
+        };
+    };
+
+  - |
+    dprx@c0064000 {
+        compatible = "intel,dprx";
+        reg = <0xc0064000 0x800>,
+              <0xc0060fe0 0x10>;
+        interrupts = <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>;
+
+        port {
+            dprx_sst_0: endpoint {
+                remote-endpoint = <&fb_sst_0>;
+            };
+        };
+    };