Re: [PATCH v5 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 06/06/24 07:29, CK Hu (胡俊光) ha scritto:
Hi, Angelo:

On Wed, 2024-06-05 at 13:15 +0200, AngeloGioacchino Del Regno wrote:
Il 05/06/24 03:38, CK Hu (胡俊光) ha scritto:
Hi, Angelo:

On Tue, 2024-05-21 at 09:57 +0200, AngeloGioacchino Del Regno wrote:
Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths
per HW instance (so potentially up to six displays for multi-vdo SoCs).

The MMSYS or VDOSYS is always the first component in the DDP pipeline,
so it only supports an output port with multiple endpoints - where each
endpoint defines the starting point for one of the (currently three)
possible hardware paths.

Reviewed-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
Reviewed-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
Tested-by: Alexandre Mergnat <amergnat@xxxxxxxxxxxx>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
---
   .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 +++++++++++++++++++
   1 file changed, 28 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
index b3c6888c1457..0ef67ca4122b 100644
--- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
@@ -93,6 +93,34 @@ properties:
     '#reset-cells':
       const: 1
+ port:
+    $ref: /schemas/graph.yaml#/properties/port
+    description:
+      Output port node. This port connects the MMSYS/VDOSYS output to
+      the first component of one display pipeline, for example one of
+      the available OVL or RDMA blocks.
+      Some MediaTek SoCs support multiple display outputs per MMSYS.

This patch looks good to me. Just want to share another information for you.
Here is an example that mmsys/vdosys could point to the display interface node.

vdosys0: syscon@1c01a000 {
            mmsys-display-interface = <&dsi0>, <&dsi1>, <&dp_intf0>;
};
vdosys1: syscon@1c100000 {
            mmsys-display-interface = <&dp_intf1>;
};

There is no conflict that mmsys/vdosys point to first component of one display pipeline or point to display interface.
Both could co-exist.


Hey CK,

yes, this could be an alternative to the OF graphs, and I'm sure that it'd work,
even though this kind of solution would still require partial hardcoding of the
display paths up until mmsys-display-interface (so, up until DSI0, or DSI1, etc).

The problem with a solution like this is that, well, even though it would work,
even if we ignore the suboptimal partial hardcoding, OF graphs are something
generic, while the mmsys-display-interface would be a MediaTek specific/custom
property.

In the end, reusing generic kernel apis/interfaces/etc is always preferred
compared to custom solutions, especially in this case, in which the generic
stuff is on-par (or actually, depending purely on personal opinions, superior).

As for the two to co-exist, I'm not sure that this is actually needed, as the
OF graphs are already (at the end of the graph) pointing to the display interface.

In any case, just as a reminder: if there will be any need to add any custom
MediaTek specific properties later, it's ok and we can do that at any time.

The alternative solution is using OF graphs to point display interface and use MediaTek specific property to first component:

vdosys0: syscon@1c01a000 {
           ports {
                    port@0 {
                              endpoint {
                                       remote-endpoint = <&dsi0_endpoint>;
                              };
                    };
port@1 {
                              endpoint {
                                       remote-endpoint = <&dsi1_endpoint>;
                              };
                    };
port@2 {
                              endpoint {
                                       remote-endpoint = <&dp_intf0_endpoint>;
                              };
                    };
           };
display-first-component = <&ovl0>;
};

And I agree to it's better to keep only OF graphs property, so it would be

vdosys0: syscon@1c01a000 {
           ports {
                    port@0 {
                              endpoint {
                                       remote-endpoint = <&dsi0_endpoint>;
};
                    };
port@1 {
                              endpoint {
                                       remote-endpoint = <&dsi1_endpoint>;
};
                    };
port@2 {
                              endpoint {
                                       remote-endpoint = <&dp_intf0_endpoint>;
                              }
;
                    };
           };
};

Maybe we could use OF graphs for both first component and display interface and drop using MediaTek specific property.


We could, or we can simply walk through the OF Graph in the driver and get the
display interface like that, as it's board-specific ;-)

...but anyway, let's see that later: after getting this series upstreamed, I will
convert all MediaTek boards (including Chromebooks) to use the graphs instead, and
you'll see that, at least for the currently supported boards, there's no need for
any custom property.

Also, setting the DSI0/1/dpintf endpoint to VDO0 is technically wrong, as that is
supposed to be the last one, and a graph is conceptually supposed to go from the
first to the last in sequence.

*if* we will ever need (probably not) to get the VDO0 node to point directly to
the last node for whatever reason, the right way would be the first one you said,
so, mediatek,mmsys-display-interface = <&dsi0>, <&dsi1>, etc etc

...or mediatek,mmsys-possible-displays = < ... phandles >

...or anyway, many other solutions are possible - but again, I think this is not
the right time to think about that. Knowing that there are eventual solutions for
any need that might arise in the future is enough, IMO :-)

Cheers,
Angelo

Regards,
CK


Cheers!
Angelo

Regards,
CK

+    properties:
+      endpoint@0:
+        $ref: /schemas/graph.yaml#/properties/endpoint
+        description: Output to the primary display pipeline
+
+      endpoint@1:
+        $ref: /schemas/graph.yaml#/properties/endpoint
+        description: Output to the secondary display pipeline
+
+      endpoint@2:
+        $ref: /schemas/graph.yaml#/properties/endpoint
+        description: Output to the tertiary display pipeline
+
+    anyOf:
+      - required:
+          - endpoint@0
+      - required:
+          - endpoint@1
+      - required:
+          - endpoint@2
+
   required:
     - compatible
     - reg







[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux