Re: [PATCH V3 4/5] dt-bindings: media: ti: vpe: Add support for Video Input Port

On 09/09/2025 10:07, Yemike Abhilash Chandra wrote:

Titles are not sentences.

Also, drop CAPS, WE DO NOT NEED TO EMPHASIZE THAT. It is a Video Input Port.

+
+maintainers:
+  - Yemike Abhilash Chandra [off-list ref]
+
+description: |-
+  Video Input Port (VIP) can be found on devices such as DRA7xx and
+  provides the system interface and the processing capability to
+  connect parallel image-sensor as well as BT.656/1120 capable encoder
+  chip to DRA7x device.
+
+  Each VIP instance supports 2 independently configurable external
+  video input capture slices (Slice 0 and Slice 1) each providing
+  up to two video input ports (Port A and Port B).
+
+properties:
+  compatible:
+    enum:
+      - ti,dra7-vip
+
+  reg:
+    items:
+      - description: The VIP main register region
+      - description: Video Data Parser (PARSER) register region for Slice0
+      - description: Color Space Conversion (CSC) register region for Slice0
+      - description: Scaler (SC) register region for Slice0
+      - description: Video Data Parser (PARSER) register region for Slice1
+      - description: Color Space Conversion (CSC) register region for Slice1
+      - description: Scaler (SC) register region for Slice1
+      - description: Video Port Direct Memory Access (VPDMA) register region
+
+  reg-names:
+    items:
+      - const: vip
+      - const: parser0
+      - const: csc0
+      - const: sc0
+      - const: parser1
+      - const: csc1
+      - const: sc1
+      - const: vpdma
+
+  interrupts:
+    items:
+      - description: IRQ index 0 is used for Slice0 interrupts
+      - description: IRQ index 1 is used for Slice1 interrupts
+
+  ti,ctrl-module:
+    description:
+      Reference to the device control module that provides clock-edge
+      inversion control for VIP ports. These controls allow the
+      VIP to sample pixel data on the correct clock edge.
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      items:
+        - description: phandle to device control module
+        - description: offset to the CTRL_CORE_SMA_SW_1 register
+        - description: Bit field to slice 0 port A
+        - description: Bit field to slice 0 port B
+        - description: Bit field to slice 1 port A
+        - description: Bit field to slice 1 port B
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    patternProperties:
+      '^port@[0-3]$':
+        $ref: /schemas/graph.yaml#/$defs/port-base
+        unevaluatedProperties: false
+        description: |
+          Each VIP instance supports 2 independently configurable external video
+          input capture slices (Slice 0 and Slice 1) each providing up to two video
+          input ports (Port A and Port B). These ports represent the following
+          port@0 -> Slice 0 Port A
+          port@1 -> Slice 0 Port B
+          port@2 -> Slice 1 Port A
+          port@3 -> Slice 1 Port B
+
+        properties:
+          endpoint:
+            $ref: /schemas/media/video-interfaces.yaml#
+            unevaluatedProperties: false
+
+            properties:
+              bus-width:
+                enum: [8, 16, 24]
+                default: 8
+
+              ti,vip-pixel-mux:
+                type: boolean
+                description:
+                  In BT656/1120 mode, this will enable pixel-muxing if
+                  the number of channels is either 1, 2 or 4. If this
+                  property is present then pixel-muxing is enabled
+                  otherwise it will use line-muxing.

This feels like runtime choice.

+
+              ti,vip-channels:
+                $ref: /schemas/types.yaml#/definitions/uint8-array
+                minItems: 1
+                maxItems: 16
+                description:
+                  In BT656/1120 mode, list of channel ids to be captured.
+                  If the property is not present then 1 channel is assumed.

Also feels like runtime.

+
+              remote-endpoint: true

Drop

+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - ti,ctrl-module
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    vip1: video@48970000 {
+        compatible = "ti,dra7-vip";
+        reg = <0x48970000 0x114>,
+              <0x48975500 0xD8>,
+              <0x48975700 0x18>,
+              <0x48975800 0x80>,
+              <0x48975a00 0xD8>,
+              <0x48975c00 0x18>,
+              <0x48975d00 0x80>,

Are you really, REALLY sure these are separate address spaces? Some
people thing that gaps means address space finishes...


How does your datasheet define these?

+              <0x4897d000 0x400>;
+        reg-names = "vip",
+                    "parser0",
+                    "csc0",
+                    "sc0",
+                    "parser1",
+                    "csc1",
+                    "sc1",
+                    "vpdma";
+        interrupts = <GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 392 IRQ_TYPE_LEVEL_HIGH>;
+        ti,ctrl-module = <&scm_conf 0x534 0x0 0x2 0x1 0x3>;
+
+        ports {
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              vin1a: port@0 {
+                    reg = <0>;
+
+                    vin1a_ep: endpoint {
+                           remote-endpoint = <&camera1>;
+                           hsync-active = <1>;
+                           vsync-active = <1>;
+                           pclk-sample = <0>;
+                           bus-width = <8>;

Make example complete, also other properties.

+                    };
+              };
+              vin1b: port@1 {
+                    reg = <1>;

Why no endpoints?

+              };
+              vin2a: port@2 {
+                    reg = <2>;
+              };
+              vin2b: port@3 {
+                    reg = <3>;

Best regards,
Krzysztof