| Message ID | 20241014085148.71105-2-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/mediatek: Add support for OF graphs | expand |
On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > The display IPs in MediaTek SoCs support being interconnected with > different instances of DDP IPs (for example, merge0 or merge1) and/or > with different DDP IPs (for example, rdma can be connected with either > color, dpi, dsi, merge, etc), forming a full Display Data Path that > ends with an actual display. > > The final display pipeline is effectively board specific, as it does > depend on the display that is attached to it, and eventually on the > sensors supported by the board (for example, Adaptive Ambient Light > would need an Ambient Light Sensor, otherwise it's pointless!), other > than the output type. > > Add support for OF graphs to most of the MediaTek DDP (display) bindings > to add flexibility to build custom hardware paths, hence enabling board > specific configuration of the display pipeline and allowing to finally > migrate away from using hardcoded paths. > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > Reviewed-by: CK Hu <ck.hu@mediatek.com> > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ > .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ > .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- > .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ > .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- > .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ > .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ > .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ > .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ > .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ > 16 files changed, 372 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > index cf24434854ff..47ddba5c41af 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > @@ -62,6 +62,27 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle-array > maxItems: 1 > > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + description: > + Input and output ports can have multiple endpoints, each of those > + connects to either the primary, secondary, etc, display pipeline. > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: AAL input port > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + AAL output to the next component's input, for example could be one > + of many gamma, overdrive or other blocks. > + > + required: > + - port@0 > + - port@1 > + > required: > - compatible > - reg > @@ -89,5 +110,24 @@ examples: > power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; > clocks = <&mmsys CLK_MM_DISP_AAL>; > mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + aal0_in: endpoint { > + remote-endpoint = <&ccorr0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + aal0_out: endpoint { > + remote-endpoint = <&gamma0_in>; > + }; > + }; > + }; > }; > }; > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > index 9f8366763831..fca8e7bb0cbc 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > @@ -57,6 +57,27 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle-array > maxItems: 1 > > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + description: > + Input and output ports can have multiple endpoints, each of those > + connects to either the primary, secondary, etc, display pipeline. > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: CCORR input port > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + CCORR output to the input of the next desired component in the > + display pipeline, usually only one of the available AAL blocks. > + > + required: > + - port@0 > + - port@1 > + > required: > - compatible > - reg > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > index 7df786bbad20..6160439ce4d7 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > @@ -65,6 +65,28 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle-array > maxItems: 1 > > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + description: > + Input and output ports can have multiple endpoints, each of those > + connects to either the primary, secondary, etc, display pipeline. > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: COLOR input port > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + COLOR output to the input of the next desired component in the > + display pipeline, for example one of the available CCORR or AAL > + blocks. > + > + required: > + - port@0 > + - port@1 > + > required: > - compatible > - reg > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > index 6fceb1f95d2a..abaf27916d13 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > @@ -56,6 +56,28 @@ properties: > $ref: /schemas/types.yaml#/definitions/phandle-array > maxItems: 1 > > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + description: > + Input and output ports can have multiple endpoints, each of those > + connects to either the primary, secondary, etc, display pipeline. > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: DITHER input, usually from a POSTMASK or GAMMA block. > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: > + DITHER output to the input of the next desired component in the > + display pipeline, for example one of the available DSC compressors, > + DP_INTF, DSI, LVDS or others. > + > + required: > + - port@0 > + - port@1 > + > required: > - compatible > - reg > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > index 3a82aec9021c..b567e3d58aa1 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > @@ -71,13 +71,34 @@ properties: > Output port node. This port should be connected to the input port of an > attached HDMI, LVDS or DisplayPort encoder chip. > > + ports: > + $ref: /schemas/graph.yaml#/properties/ports > + > + properties: > + port@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: DPI input port > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: DPI output to an HDMI, LVDS or DisplayPort encoder input This is wrong. The existing 'port' is the output. 'port' and 'port@0' are treated as the same thing. Since you are adding an input port, the new port has to be 'port@1' (or any number but 0). I haven't looked at the driver code, but it should request port 0 and always get the output port. And requesting port 1 will return an error or the input port. Rob
Il 14/10/24 19:36, Rob Herring ha scritto: > On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> The display IPs in MediaTek SoCs support being interconnected with >> different instances of DDP IPs (for example, merge0 or merge1) and/or >> with different DDP IPs (for example, rdma can be connected with either >> color, dpi, dsi, merge, etc), forming a full Display Data Path that >> ends with an actual display. >> >> The final display pipeline is effectively board specific, as it does >> depend on the display that is attached to it, and eventually on the >> sensors supported by the board (for example, Adaptive Ambient Light >> would need an Ambient Light Sensor, otherwise it's pointless!), other >> than the output type. >> >> Add support for OF graphs to most of the MediaTek DDP (display) bindings >> to add flexibility to build custom hardware paths, hence enabling board >> specific configuration of the display pipeline and allowing to finally >> migrate away from using hardcoded paths. >> >> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> >> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >> Reviewed-by: CK Hu <ck.hu@mediatek.com> >> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ >> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ >> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- >> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ >> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- >> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ >> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ >> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ >> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ >> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ >> 16 files changed, 372 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >> index cf24434854ff..47ddba5c41af 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >> @@ -62,6 +62,27 @@ properties: >> $ref: /schemas/types.yaml#/definitions/phandle-array >> maxItems: 1 >> >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + description: >> + Input and output ports can have multiple endpoints, each of those >> + connects to either the primary, secondary, etc, display pipeline. >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: AAL input port >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + AAL output to the next component's input, for example could be one >> + of many gamma, overdrive or other blocks. >> + >> + required: >> + - port@0 >> + - port@1 >> + >> required: >> - compatible >> - reg >> @@ -89,5 +110,24 @@ examples: >> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; >> clocks = <&mmsys CLK_MM_DISP_AAL>; >> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; >> + >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + aal0_in: endpoint { >> + remote-endpoint = <&ccorr0_out>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + aal0_out: endpoint { >> + remote-endpoint = <&gamma0_in>; >> + }; >> + }; >> + }; >> }; >> }; >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >> index 9f8366763831..fca8e7bb0cbc 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >> @@ -57,6 +57,27 @@ properties: >> $ref: /schemas/types.yaml#/definitions/phandle-array >> maxItems: 1 >> >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + description: >> + Input and output ports can have multiple endpoints, each of those >> + connects to either the primary, secondary, etc, display pipeline. >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: CCORR input port >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + CCORR output to the input of the next desired component in the >> + display pipeline, usually only one of the available AAL blocks. >> + >> + required: >> + - port@0 >> + - port@1 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >> index 7df786bbad20..6160439ce4d7 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >> @@ -65,6 +65,28 @@ properties: >> $ref: /schemas/types.yaml#/definitions/phandle-array >> maxItems: 1 >> >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + description: >> + Input and output ports can have multiple endpoints, each of those >> + connects to either the primary, secondary, etc, display pipeline. >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: COLOR input port >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + COLOR output to the input of the next desired component in the >> + display pipeline, for example one of the available CCORR or AAL >> + blocks. >> + >> + required: >> + - port@0 >> + - port@1 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >> index 6fceb1f95d2a..abaf27916d13 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >> @@ -56,6 +56,28 @@ properties: >> $ref: /schemas/types.yaml#/definitions/phandle-array >> maxItems: 1 >> >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + description: >> + Input and output ports can have multiple endpoints, each of those >> + connects to either the primary, secondary, etc, display pipeline. >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: DITHER input, usually from a POSTMASK or GAMMA block. >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: >> + DITHER output to the input of the next desired component in the >> + display pipeline, for example one of the available DSC compressors, >> + DP_INTF, DSI, LVDS or others. >> + >> + required: >> + - port@0 >> + - port@1 >> + >> required: >> - compatible >> - reg >> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >> index 3a82aec9021c..b567e3d58aa1 100644 >> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >> @@ -71,13 +71,34 @@ properties: >> Output port node. This port should be connected to the input port of an >> attached HDMI, LVDS or DisplayPort encoder chip. >> >> + ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + >> + properties: >> + port@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: DPI input port >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input > > This is wrong. The existing 'port' is the output. 'port' and 'port@0' > are treated as the same thing. Since you are adding an input port, the > new port has to be 'port@1' (or any number but 0). > > I haven't looked at the driver code, but it should request port 0 and > always get the output port. And requesting port 1 will return an error > or the input port. Hello Rob, I want to remind you that in v2 of this series you said that it'd be wrong for port@0 to be an output, I replied that you misread that as I had modeled it indeed as an input, and then you gave me your Reviewed-by tag. Anyway - I get your concern about the previous behavior of `port`, but I chose to model this that way purely for consistency. First of all - the driver(s) will check if we're feeding a full graph, as it will indeed first check if port@1 is present: if it is, then it follows this scheme with port@0 as INPUT and port@1 as OUTPUT. If the component in port@0 is an OUTPUT, the bridge attach will fail. Getting to bindings themselves, then... it would be a mistake to model port@0 as an output and port@1 as an input, because that would be not only inconsistent with the DRM Bridge bindings, but would be highly confusing when reading the devicetree. Please note that the bridge bindings are always declaring port@0 as an INPUT and other ports as OUTPUT(s). As an example, you can check display/bridge/analogix,anx7625.yaml or display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml (and others) for display controllers, which do all conform to this logic, where the input is always @0, and the output is @1. Of course, doing this required me to do extra changes to the MTK DRM drivers to actually be retro-compatible with the old devicetrees as I explained before. Just for clarity, if I were to model this with port@0 OUTPUT and @1 INPUT, we would see in devicetree something like: dpi-node@somewhere { ports { some_output_1: port@0 { remote-endpoint = <&some_input_2>; }; some_input_1: port@1 { remote-endpoint = <&some_output_0>; }; }; /* already existing bridge binding, not touched by this commit */ bridge@somewhere-else { ports { some_input_2: port@0 { remote-endpoint = <&some_output_1>; }; some_output_2: port@1 { remote-endpoint = <&to_display_input>; }; }; ...and I think that you agree with me that this would be at least confusing for whoever reads the DT (and again, IMO, inconsistent and simply wrong). Instead, with the model proposed in this commit, we will have consistency: dpi-node@somewhere { ports { some_input_1: port@1 { remote-endpoint = <&some_output_0>; }; some_output_1: port@0 { remote-endpoint = <&some_input_2>; }; }; /* already existing bridge binding, not touched by this commit */ bridge@somewhere-else { ports { some_input_2: port@0 { remote-endpoint = <&some_output_1>; }; some_output_2: port@1 { remote-endpoint = <&to_display_input>; }; }; ...still, again, all this while still supporting the old device trees (which I plan to update as soon as possible anyway, so that they're all using the full graph instead of hardcoding board specific paths in the drivers). Does this clarify to you the reasons why this was done like that? If you have any other questions, I will be happy to clarify. Cheers, Angelo
On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: > Il 14/10/24 19:36, Rob Herring ha scritto: > > On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > > > > > > The display IPs in MediaTek SoCs support being interconnected with > > > different instances of DDP IPs (for example, merge0 or merge1) and/or > > > with different DDP IPs (for example, rdma can be connected with either > > > color, dpi, dsi, merge, etc), forming a full Display Data Path that > > > ends with an actual display. > > > > > > The final display pipeline is effectively board specific, as it does > > > depend on the display that is attached to it, and eventually on the > > > sensors supported by the board (for example, Adaptive Ambient Light > > > would need an Ambient Light Sensor, otherwise it's pointless!), other > > > than the output type. > > > > > > Add support for OF graphs to most of the MediaTek DDP (display) bindings > > > to add flexibility to build custom hardware paths, hence enabling board > > > specific configuration of the display pipeline and allowing to finally > > > migrate away from using hardcoded paths. > > > > > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > > > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > > > Reviewed-by: CK Hu <ck.hu@mediatek.com> > > > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > --- > > > .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ > > > .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ > > > .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- > > > .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ > > > .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- > > > .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ > > > .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ > > > .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ > > > .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ > > > .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ > > > 16 files changed, 372 insertions(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > > > index cf24434854ff..47ddba5c41af 100644 > > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > > > @@ -62,6 +62,27 @@ properties: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > maxItems: 1 > > > > > > + ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + description: > > > + Input and output ports can have multiple endpoints, each of those > > > + connects to either the primary, secondary, etc, display pipeline. > > > + > > > + properties: > > > + port@0: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: AAL input port > > > + > > > + port@1: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: > > > + AAL output to the next component's input, for example could be one > > > + of many gamma, overdrive or other blocks. > > > + > > > + required: > > > + - port@0 > > > + - port@1 > > > + > > > required: > > > - compatible > > > - reg > > > @@ -89,5 +110,24 @@ examples: > > > power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; > > > clocks = <&mmsys CLK_MM_DISP_AAL>; > > > mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; > > > + > > > + ports { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + port@0 { > > > + reg = <0>; > > > + aal0_in: endpoint { > > > + remote-endpoint = <&ccorr0_out>; > > > + }; > > > + }; > > > + > > > + port@1 { > > > + reg = <1>; > > > + aal0_out: endpoint { > > > + remote-endpoint = <&gamma0_in>; > > > + }; > > > + }; > > > + }; > > > }; > > > }; > > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > > > index 9f8366763831..fca8e7bb0cbc 100644 > > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > > > @@ -57,6 +57,27 @@ properties: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > maxItems: 1 > > > > > > + ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + description: > > > + Input and output ports can have multiple endpoints, each of those > > > + connects to either the primary, secondary, etc, display pipeline. > > > + > > > + properties: > > > + port@0: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: CCORR input port > > > + > > > + port@1: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: > > > + CCORR output to the input of the next desired component in the > > > + display pipeline, usually only one of the available AAL blocks. > > > + > > > + required: > > > + - port@0 > > > + - port@1 > > > + > > > required: > > > - compatible > > > - reg > > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > > > index 7df786bbad20..6160439ce4d7 100644 > > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > > > @@ -65,6 +65,28 @@ properties: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > maxItems: 1 > > > > > > + ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + description: > > > + Input and output ports can have multiple endpoints, each of those > > > + connects to either the primary, secondary, etc, display pipeline. > > > + > > > + properties: > > > + port@0: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: COLOR input port > > > + > > > + port@1: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: > > > + COLOR output to the input of the next desired component in the > > > + display pipeline, for example one of the available CCORR or AAL > > > + blocks. > > > + > > > + required: > > > + - port@0 > > > + - port@1 > > > + > > > required: > > > - compatible > > > - reg > > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > > > index 6fceb1f95d2a..abaf27916d13 100644 > > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > > > @@ -56,6 +56,28 @@ properties: > > > $ref: /schemas/types.yaml#/definitions/phandle-array > > > maxItems: 1 > > > > > > + ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + description: > > > + Input and output ports can have multiple endpoints, each of those > > > + connects to either the primary, secondary, etc, display pipeline. > > > + > > > + properties: > > > + port@0: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: DITHER input, usually from a POSTMASK or GAMMA block. > > > + > > > + port@1: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: > > > + DITHER output to the input of the next desired component in the > > > + display pipeline, for example one of the available DSC compressors, > > > + DP_INTF, DSI, LVDS or others. > > > + > > > + required: > > > + - port@0 > > > + - port@1 > > > + > > > required: > > > - compatible > > > - reg > > > diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > > > index 3a82aec9021c..b567e3d58aa1 100644 > > > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > > > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > > > @@ -71,13 +71,34 @@ properties: > > > Output port node. This port should be connected to the input port of an > > > attached HDMI, LVDS or DisplayPort encoder chip. > > > > > > + ports: > > > + $ref: /schemas/graph.yaml#/properties/ports > > > + > > > + properties: > > > + port@0: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: DPI input port > > > + > > > + port@1: > > > + $ref: /schemas/graph.yaml#/properties/port > > > + description: DPI output to an HDMI, LVDS or DisplayPort encoder input > > > > This is wrong. The existing 'port' is the output. 'port' and 'port@0' > > are treated as the same thing. Since you are adding an input port, the > > new port has to be 'port@1' (or any number but 0). > > > > I haven't looked at the driver code, but it should request port 0 and > > always get the output port. And requesting port 1 will return an error > > or the input port. > > Hello Rob, > > I want to remind you that in v2 of this series you said that it'd be wrong for > port@0 to be an output, I replied that you misread that as I had modeled it indeed > as an input, and then you gave me your Reviewed-by tag. I have not misread it. Then I guess I forgot about it and missed it the next time on v3. > Anyway - I get your concern about the previous behavior of `port`, but I chose to > model this that way purely for consistency. > > First of all - the driver(s) will check if we're feeding a full graph, as it will > indeed first check if port@1 is present: if it is, then it follows this scheme with > port@0 as INPUT and port@1 as OUTPUT. > If the component in port@0 is an OUTPUT, the bridge attach will fail. > > Getting to bindings themselves, then... it would be a mistake to model port@0 as an > output and port@1 as an input, because that would be not only inconsistent with the > DRM Bridge bindings, but would be highly confusing when reading the devicetree. Somewhat confusing, yes. Highly, no. Put a comment in the DT. > Please note that the bridge bindings are always declaring port@0 as an INPUT and > other ports as OUTPUT(s). There is no guarantee on that. Port numbering is local to the bridge and opaque to anything outside that bridge. That is why you have to document what each port represents. Would we have followed that convention if all the ports were defined from the start? Certainly. But that didn't happen and you are stuck with the existing binding and ABI. > As an example, you can check display/bridge/analogix,anx7625.yaml or > display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise > display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml > (and others) for display controllers, which do all conform to this logic, where > the input is always @0, and the output is @1. > > Of course, doing this required me to do extra changes to the MTK DRM drivers to > actually be retro-compatible with the old devicetrees as I explained before. You can't fix existing software... If you want to break the ABI, then that's ultimately up to you and Mediatek maintainers to decide0. This case is easy to avoid, why would you knowingly break the ABI here. OTOH, this seems like a big enough change I would imagine it is a matter of time before supporting a missing OF graph for the components will be a problem. Rob
Il 15/10/24 15:48, Rob Herring ha scritto: > On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: >> Il 14/10/24 19:36, Rob Herring ha scritto: >>> On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> wrote: >>>> >>>> The display IPs in MediaTek SoCs support being interconnected with >>>> different instances of DDP IPs (for example, merge0 or merge1) and/or >>>> with different DDP IPs (for example, rdma can be connected with either >>>> color, dpi, dsi, merge, etc), forming a full Display Data Path that >>>> ends with an actual display. >>>> >>>> The final display pipeline is effectively board specific, as it does >>>> depend on the display that is attached to it, and eventually on the >>>> sensors supported by the board (for example, Adaptive Ambient Light >>>> would need an Ambient Light Sensor, otherwise it's pointless!), other >>>> than the output type. >>>> >>>> Add support for OF graphs to most of the MediaTek DDP (display) bindings >>>> to add flexibility to build custom hardware paths, hence enabling board >>>> specific configuration of the display pipeline and allowing to finally >>>> migrate away from using hardcoded paths. >>>> >>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> >>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> Reviewed-by: CK Hu <ck.hu@mediatek.com> >>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>> --- >>>> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ >>>> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ >>>> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- >>>> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ >>>> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- >>>> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ >>>> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ >>>> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ >>>> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ >>>> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ >>>> 16 files changed, 372 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>> index cf24434854ff..47ddba5c41af 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>> @@ -62,6 +62,27 @@ properties: >>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>> maxItems: 1 >>>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + description: >>>> + Input and output ports can have multiple endpoints, each of those >>>> + connects to either the primary, secondary, etc, display pipeline. >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: AAL input port >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: >>>> + AAL output to the next component's input, for example could be one >>>> + of many gamma, overdrive or other blocks. >>>> + >>>> + required: >>>> + - port@0 >>>> + - port@1 >>>> + >>>> required: >>>> - compatible >>>> - reg >>>> @@ -89,5 +110,24 @@ examples: >>>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; >>>> clocks = <&mmsys CLK_MM_DISP_AAL>; >>>> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; >>>> + >>>> + ports { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + port@0 { >>>> + reg = <0>; >>>> + aal0_in: endpoint { >>>> + remote-endpoint = <&ccorr0_out>; >>>> + }; >>>> + }; >>>> + >>>> + port@1 { >>>> + reg = <1>; >>>> + aal0_out: endpoint { >>>> + remote-endpoint = <&gamma0_in>; >>>> + }; >>>> + }; >>>> + }; >>>> }; >>>> }; >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>> index 9f8366763831..fca8e7bb0cbc 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>> @@ -57,6 +57,27 @@ properties: >>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>> maxItems: 1 >>>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + description: >>>> + Input and output ports can have multiple endpoints, each of those >>>> + connects to either the primary, secondary, etc, display pipeline. >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: CCORR input port >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: >>>> + CCORR output to the input of the next desired component in the >>>> + display pipeline, usually only one of the available AAL blocks. >>>> + >>>> + required: >>>> + - port@0 >>>> + - port@1 >>>> + >>>> required: >>>> - compatible >>>> - reg >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>> index 7df786bbad20..6160439ce4d7 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>> @@ -65,6 +65,28 @@ properties: >>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>> maxItems: 1 >>>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + description: >>>> + Input and output ports can have multiple endpoints, each of those >>>> + connects to either the primary, secondary, etc, display pipeline. >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: COLOR input port >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: >>>> + COLOR output to the input of the next desired component in the >>>> + display pipeline, for example one of the available CCORR or AAL >>>> + blocks. >>>> + >>>> + required: >>>> + - port@0 >>>> + - port@1 >>>> + >>>> required: >>>> - compatible >>>> - reg >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>> index 6fceb1f95d2a..abaf27916d13 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>> @@ -56,6 +56,28 @@ properties: >>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>> maxItems: 1 >>>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + description: >>>> + Input and output ports can have multiple endpoints, each of those >>>> + connects to either the primary, secondary, etc, display pipeline. >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: DITHER input, usually from a POSTMASK or GAMMA block. >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: >>>> + DITHER output to the input of the next desired component in the >>>> + display pipeline, for example one of the available DSC compressors, >>>> + DP_INTF, DSI, LVDS or others. >>>> + >>>> + required: >>>> + - port@0 >>>> + - port@1 >>>> + >>>> required: >>>> - compatible >>>> - reg >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>> index 3a82aec9021c..b567e3d58aa1 100644 >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>> @@ -71,13 +71,34 @@ properties: >>>> Output port node. This port should be connected to the input port of an >>>> attached HDMI, LVDS or DisplayPort encoder chip. >>>> >>>> + ports: >>>> + $ref: /schemas/graph.yaml#/properties/ports >>>> + >>>> + properties: >>>> + port@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: DPI input port >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input >>> >>> This is wrong. The existing 'port' is the output. 'port' and 'port@0' >>> are treated as the same thing. Since you are adding an input port, the >>> new port has to be 'port@1' (or any number but 0). >>> >>> I haven't looked at the driver code, but it should request port 0 and >>> always get the output port. And requesting port 1 will return an error >>> or the input port. >> >> Hello Rob, >> >> I want to remind you that in v2 of this series you said that it'd be wrong for >> port@0 to be an output, I replied that you misread that as I had modeled it indeed >> as an input, and then you gave me your Reviewed-by tag. > > I have not misread it. Then I guess I forgot about it and missed it the > next time on v3. > Okay, that was some misunderstanding then - it's fine, no problem. >> Anyway - I get your concern about the previous behavior of `port`, but I chose to >> model this that way purely for consistency. >> >> First of all - the driver(s) will check if we're feeding a full graph, as it will >> indeed first check if port@1 is present: if it is, then it follows this scheme with >> port@0 as INPUT and port@1 as OUTPUT. >> If the component in port@0 is an OUTPUT, the bridge attach will fail. >> >> Getting to bindings themselves, then... it would be a mistake to model port@0 as an >> output and port@1 as an input, because that would be not only inconsistent with the >> DRM Bridge bindings, but would be highly confusing when reading the devicetree. > > Somewhat confusing, yes. Highly, no. Put a comment in the DT. > "Somewhat or highly" boils down to personal opinion, so I still go for "highly" but won't argue about that as wouldn't be productive and would bring us nowhere anyway, so, whatever :-) Putting a comment in DT is an option, yes, but that comment would need to be put on all of the MediaTek SoC device trees - current and future - and IMO would scream "inconsistency warning" (in different words, of course) all over, which honestly doesn't really look clean... at least to my eyes... >> Please note that the bridge bindings are always declaring port@0 as an INPUT and >> other ports as OUTPUT(s). > > There is no guarantee on that. Port numbering is local to the bridge and > opaque to anything outside that bridge. That is why you have to document > what each port represents. > I know and I agree that there's no guarantee on the numbering. I can see that in other non-drm-bridge bindings, and that's fine. > Would we have followed that convention if all the ports were defined > from the start? Certainly. But that didn't happen and you are stuck with > the existing binding and ABI. > I thought about adding a new compatible for the new port scheme, but that looked even worse to me as, after doing that (yeah, I actually went for it in the first version that I have never sent upstream) during my own proof-read I started screaming "HACK! HACK! NOOO!" all over, and rewritten the entire thing. >> As an example, you can check display/bridge/analogix,anx7625.yaml or >> display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise >> display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml >> (and others) for display controllers, which do all conform to this logic, where >> the input is always @0, and the output is @1. >> >> Of course, doing this required me to do extra changes to the MTK DRM drivers to >> actually be retro-compatible with the old devicetrees as I explained before. > > You can't fix existing software... > That's true, but I don't see that as an "excuse" (grant me this term please) to "carelessly" keep it in a suboptimal state. This driver has been growing almost uncontrollably with (wrong, anyway!) board-specific component vectors - and writing a new one would just add up more code duplication to the mix and/or worsen the maintainability of older MediaTek SoCs (as the "old" driver will get forgotten and never updated anymore). > If you want to break the ABI, then that's ultimately up to you and > Mediatek maintainers to decide0. This case is easy to avoid, why would > you knowingly break the ABI here. Because if we don't do this, we condemn (forever) this driver, or part of it to have an inverted port scheme compared to either: A. The other drm/mediatek components; or B. The other bridge drivers (of which, some are used with MTK as well) ...and we also condemn (forever, again) all MediaTek device trees to scream "port inconsistency warning: A for drm/mediatek components, B for every other thing", which would scream "drm/mediatek is somewhat broken", which can be avoided. > OTOH, this seems like a big enough > change I would imagine it is a matter of time before supporting a > missing OF graph for the components will be a problem. > Sorry I didn't understand this part, can you please-please-please reword? Cheers, Angelo
On Wed, Oct 16, 2024 at 4:23 AM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 15/10/24 15:48, Rob Herring ha scritto: > > On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: > >> Il 14/10/24 19:36, Rob Herring ha scritto: > >>> On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno > >>> <angelogioacchino.delregno@collabora.com> wrote: > >>>> > >>>> The display IPs in MediaTek SoCs support being interconnected with > >>>> different instances of DDP IPs (for example, merge0 or merge1) and/or > >>>> with different DDP IPs (for example, rdma can be connected with either > >>>> color, dpi, dsi, merge, etc), forming a full Display Data Path that > >>>> ends with an actual display. > >>>> > >>>> The final display pipeline is effectively board specific, as it does > >>>> depend on the display that is attached to it, and eventually on the > >>>> sensors supported by the board (for example, Adaptive Ambient Light > >>>> would need an Ambient Light Sensor, otherwise it's pointless!), other > >>>> than the output type. > >>>> > >>>> Add support for OF graphs to most of the MediaTek DDP (display) bindings > >>>> to add flexibility to build custom hardware paths, hence enabling board > >>>> specific configuration of the display pipeline and allowing to finally > >>>> migrate away from using hardcoded paths. > >>>> > >>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > >>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > >>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > >>>> Reviewed-by: CK Hu <ck.hu@mediatek.com> > >>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > >>>> --- > >>>> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ > >>>> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ > >>>> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- > >>>> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ > >>>> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- > >>>> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ > >>>> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ > >>>> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ > >>>> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ > >>>> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ > >>>> 16 files changed, 372 insertions(+), 3 deletions(-) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>> index cf24434854ff..47ddba5c41af 100644 > >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>> @@ -62,6 +62,27 @@ properties: > >>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>> maxItems: 1 > >>>> > >>>> + ports: > >>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>> + description: > >>>> + Input and output ports can have multiple endpoints, each of those > >>>> + connects to either the primary, secondary, etc, display pipeline. > >>>> + > >>>> + properties: > >>>> + port@0: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: AAL input port > >>>> + > >>>> + port@1: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: > >>>> + AAL output to the next component's input, for example could be one > >>>> + of many gamma, overdrive or other blocks. > >>>> + > >>>> + required: > >>>> + - port@0 > >>>> + - port@1 > >>>> + > >>>> required: > >>>> - compatible > >>>> - reg > >>>> @@ -89,5 +110,24 @@ examples: > >>>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; > >>>> clocks = <&mmsys CLK_MM_DISP_AAL>; > >>>> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; > >>>> + > >>>> + ports { > >>>> + #address-cells = <1>; > >>>> + #size-cells = <0>; > >>>> + > >>>> + port@0 { > >>>> + reg = <0>; > >>>> + aal0_in: endpoint { > >>>> + remote-endpoint = <&ccorr0_out>; > >>>> + }; > >>>> + }; > >>>> + > >>>> + port@1 { > >>>> + reg = <1>; > >>>> + aal0_out: endpoint { > >>>> + remote-endpoint = <&gamma0_in>; > >>>> + }; > >>>> + }; > >>>> + }; > >>>> }; > >>>> }; > >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>> index 9f8366763831..fca8e7bb0cbc 100644 > >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>> @@ -57,6 +57,27 @@ properties: > >>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>> maxItems: 1 > >>>> > >>>> + ports: > >>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>> + description: > >>>> + Input and output ports can have multiple endpoints, each of those > >>>> + connects to either the primary, secondary, etc, display pipeline. > >>>> + > >>>> + properties: > >>>> + port@0: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: CCORR input port > >>>> + > >>>> + port@1: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: > >>>> + CCORR output to the input of the next desired component in the > >>>> + display pipeline, usually only one of the available AAL blocks. > >>>> + > >>>> + required: > >>>> + - port@0 > >>>> + - port@1 > >>>> + > >>>> required: > >>>> - compatible > >>>> - reg > >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>> index 7df786bbad20..6160439ce4d7 100644 > >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>> @@ -65,6 +65,28 @@ properties: > >>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>> maxItems: 1 > >>>> > >>>> + ports: > >>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>> + description: > >>>> + Input and output ports can have multiple endpoints, each of those > >>>> + connects to either the primary, secondary, etc, display pipeline. > >>>> + > >>>> + properties: > >>>> + port@0: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: COLOR input port > >>>> + > >>>> + port@1: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: > >>>> + COLOR output to the input of the next desired component in the > >>>> + display pipeline, for example one of the available CCORR or AAL > >>>> + blocks. > >>>> + > >>>> + required: > >>>> + - port@0 > >>>> + - port@1 > >>>> + > >>>> required: > >>>> - compatible > >>>> - reg > >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>> index 6fceb1f95d2a..abaf27916d13 100644 > >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>> @@ -56,6 +56,28 @@ properties: > >>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>> maxItems: 1 > >>>> > >>>> + ports: > >>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>> + description: > >>>> + Input and output ports can have multiple endpoints, each of those > >>>> + connects to either the primary, secondary, etc, display pipeline. > >>>> + > >>>> + properties: > >>>> + port@0: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: DITHER input, usually from a POSTMASK or GAMMA block. > >>>> + > >>>> + port@1: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: > >>>> + DITHER output to the input of the next desired component in the > >>>> + display pipeline, for example one of the available DSC compressors, > >>>> + DP_INTF, DSI, LVDS or others. > >>>> + > >>>> + required: > >>>> + - port@0 > >>>> + - port@1 > >>>> + > >>>> required: > >>>> - compatible > >>>> - reg > >>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>> index 3a82aec9021c..b567e3d58aa1 100644 > >>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>> @@ -71,13 +71,34 @@ properties: > >>>> Output port node. This port should be connected to the input port of an > >>>> attached HDMI, LVDS or DisplayPort encoder chip. > >>>> > >>>> + ports: > >>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>> + > >>>> + properties: > >>>> + port@0: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: DPI input port > >>>> + > >>>> + port@1: > >>>> + $ref: /schemas/graph.yaml#/properties/port > >>>> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input > >>> > >>> This is wrong. The existing 'port' is the output. 'port' and 'port@0' > >>> are treated as the same thing. Since you are adding an input port, the > >>> new port has to be 'port@1' (or any number but 0). > >>> > >>> I haven't looked at the driver code, but it should request port 0 and > >>> always get the output port. And requesting port 1 will return an error > >>> or the input port. > >> > >> Hello Rob, > >> > >> I want to remind you that in v2 of this series you said that it'd be wrong for > >> port@0 to be an output, I replied that you misread that as I had modeled it indeed > >> as an input, and then you gave me your Reviewed-by tag. > > > > I have not misread it. Then I guess I forgot about it and missed it the > > next time on v3. > > > > Okay, that was some misunderstanding then - it's fine, no problem. > > >> Anyway - I get your concern about the previous behavior of `port`, but I chose to > >> model this that way purely for consistency. > >> > >> First of all - the driver(s) will check if we're feeding a full graph, as it will > >> indeed first check if port@1 is present: if it is, then it follows this scheme with > >> port@0 as INPUT and port@1 as OUTPUT. > >> If the component in port@0 is an OUTPUT, the bridge attach will fail. > >> > >> Getting to bindings themselves, then... it would be a mistake to model port@0 as an > >> output and port@1 as an input, because that would be not only inconsistent with the > >> DRM Bridge bindings, but would be highly confusing when reading the devicetree. > > > > Somewhat confusing, yes. Highly, no. Put a comment in the DT. > > > > "Somewhat or highly" boils down to personal opinion, so I still go for "highly" > but won't argue about that as wouldn't be productive and would bring us nowhere > anyway, so, whatever :-) > > Putting a comment in DT is an option, yes, but that comment would need to be put > on all of the MediaTek SoC device trees - current and future - and IMO would scream > "inconsistency warning" (in different words, of course) all over, which honestly > doesn't really look clean... at least to my eyes... What I find more confusing is my updated DT doesn't work with my existing kernel... > >> Please note that the bridge bindings are always declaring port@0 as an INPUT and > >> other ports as OUTPUT(s). > > > > There is no guarantee on that. Port numbering is local to the bridge and > > opaque to anything outside that bridge. That is why you have to document > > what each port represents. > > > > I know and I agree that there's no guarantee on the numbering. I can see that in > other non-drm-bridge bindings, and that's fine. > > > Would we have followed that convention if all the ports were defined > > from the start? Certainly. But that didn't happen and you are stuck with > > the existing binding and ABI. > > > > I thought about adding a new compatible for the new port scheme, but that looked > even worse to me as, after doing that (yeah, I actually went for it in the first > version that I have never sent upstream) during my own proof-read I started > screaming "HACK! HACK! NOOO!" all over, and rewritten the entire thing. > > >> As an example, you can check display/bridge/analogix,anx7625.yaml or > >> display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise > >> display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml > >> (and others) for display controllers, which do all conform to this logic, where > >> the input is always @0, and the output is @1. > >> > >> Of course, doing this required me to do extra changes to the MTK DRM drivers to > >> actually be retro-compatible with the old devicetrees as I explained before. > > > > You can't fix existing software... > > > > That's true, but I don't see that as an "excuse" (grant me this term please) to > "carelessly" keep it in a suboptimal state. > > This driver has been growing almost uncontrollably with (wrong, anyway!) > board-specific component vectors - and writing a new one would just add up > more code duplication to the mix and/or worsen the maintainability of older > MediaTek SoCs (as the "old" driver will get forgotten and never updated anymore). > > > If you want to break the ABI, then that's ultimately up to you and > > Mediatek maintainers to decide0. This case is easy to avoid, why would > > you knowingly break the ABI here. > > Because if we don't do this, we condemn (forever) this driver, or part of it > to have an inverted port scheme compared to either: > A. The other drm/mediatek components; or > B. The other bridge drivers (of which, some are used with MTK as well) > > ...and we also condemn (forever, again) all MediaTek device trees to scream > "port inconsistency warning: A for drm/mediatek components, B for every other > thing", which would scream "drm/mediatek is somewhat broken", which can be > avoided. Sure. The cost is just an ABI break to do that. > > OTOH, this seems like a big enough > > change I would imagine it is a matter of time before supporting a > > missing OF graph for the components will be a problem. > > > > Sorry I didn't understand this part, can you please-please-please reword? I think keeping the kernel working with the old and new binding will be a challenge. Partly because testing with the old binding won't happen, but also if the binding and drivers continue to evolve. So while maintaining old ABI might be possible with this change, it will continue to be an issue with each change. BTW, did you actually test backwards compatibility with this? I can see you fallback to the old binding, but there's a lot of other changes in there I can't really tell by looking at it. What I haven't heard from you is "yes, we need to maintain the ABI" or "no, we can break it". Decide that, then the path here is simple. Rob
Il 16/10/24 16:00, Rob Herring ha scritto: > On Wed, Oct 16, 2024 at 4:23 AM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 15/10/24 15:48, Rob Herring ha scritto: >>> On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: >>>> Il 14/10/24 19:36, Rob Herring ha scritto: >>>>> On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno >>>>> <angelogioacchino.delregno@collabora.com> wrote: >>>>>> >>>>>> The display IPs in MediaTek SoCs support being interconnected with >>>>>> different instances of DDP IPs (for example, merge0 or merge1) and/or >>>>>> with different DDP IPs (for example, rdma can be connected with either >>>>>> color, dpi, dsi, merge, etc), forming a full Display Data Path that >>>>>> ends with an actual display. >>>>>> >>>>>> The final display pipeline is effectively board specific, as it does >>>>>> depend on the display that is attached to it, and eventually on the >>>>>> sensors supported by the board (for example, Adaptive Ambient Light >>>>>> would need an Ambient Light Sensor, otherwise it's pointless!), other >>>>>> than the output type. >>>>>> >>>>>> Add support for OF graphs to most of the MediaTek DDP (display) bindings >>>>>> to add flexibility to build custom hardware paths, hence enabling board >>>>>> specific configuration of the display pipeline and allowing to finally >>>>>> migrate away from using hardcoded paths. >>>>>> >>>>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> >>>>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>> Reviewed-by: CK Hu <ck.hu@mediatek.com> >>>>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>>>> --- >>>>>> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ >>>>>> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ >>>>>> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- >>>>>> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ >>>>>> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- >>>>>> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ >>>>>> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ >>>>>> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ >>>>>> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ >>>>>> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ >>>>>> 16 files changed, 372 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>> index cf24434854ff..47ddba5c41af 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>> @@ -62,6 +62,27 @@ properties: >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>> maxItems: 1 >>>>>> >>>>>> + ports: >>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>> + description: >>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>> + >>>>>> + properties: >>>>>> + port@0: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: AAL input port >>>>>> + >>>>>> + port@1: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: >>>>>> + AAL output to the next component's input, for example could be one >>>>>> + of many gamma, overdrive or other blocks. >>>>>> + >>>>>> + required: >>>>>> + - port@0 >>>>>> + - port@1 >>>>>> + >>>>>> required: >>>>>> - compatible >>>>>> - reg >>>>>> @@ -89,5 +110,24 @@ examples: >>>>>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; >>>>>> clocks = <&mmsys CLK_MM_DISP_AAL>; >>>>>> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; >>>>>> + >>>>>> + ports { >>>>>> + #address-cells = <1>; >>>>>> + #size-cells = <0>; >>>>>> + >>>>>> + port@0 { >>>>>> + reg = <0>; >>>>>> + aal0_in: endpoint { >>>>>> + remote-endpoint = <&ccorr0_out>; >>>>>> + }; >>>>>> + }; >>>>>> + >>>>>> + port@1 { >>>>>> + reg = <1>; >>>>>> + aal0_out: endpoint { >>>>>> + remote-endpoint = <&gamma0_in>; >>>>>> + }; >>>>>> + }; >>>>>> + }; >>>>>> }; >>>>>> }; >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>> index 9f8366763831..fca8e7bb0cbc 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>> @@ -57,6 +57,27 @@ properties: >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>> maxItems: 1 >>>>>> >>>>>> + ports: >>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>> + description: >>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>> + >>>>>> + properties: >>>>>> + port@0: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: CCORR input port >>>>>> + >>>>>> + port@1: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: >>>>>> + CCORR output to the input of the next desired component in the >>>>>> + display pipeline, usually only one of the available AAL blocks. >>>>>> + >>>>>> + required: >>>>>> + - port@0 >>>>>> + - port@1 >>>>>> + >>>>>> required: >>>>>> - compatible >>>>>> - reg >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>> index 7df786bbad20..6160439ce4d7 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>> @@ -65,6 +65,28 @@ properties: >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>> maxItems: 1 >>>>>> >>>>>> + ports: >>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>> + description: >>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>> + >>>>>> + properties: >>>>>> + port@0: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: COLOR input port >>>>>> + >>>>>> + port@1: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: >>>>>> + COLOR output to the input of the next desired component in the >>>>>> + display pipeline, for example one of the available CCORR or AAL >>>>>> + blocks. >>>>>> + >>>>>> + required: >>>>>> + - port@0 >>>>>> + - port@1 >>>>>> + >>>>>> required: >>>>>> - compatible >>>>>> - reg >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>> index 6fceb1f95d2a..abaf27916d13 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>> @@ -56,6 +56,28 @@ properties: >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>> maxItems: 1 >>>>>> >>>>>> + ports: >>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>> + description: >>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>> + >>>>>> + properties: >>>>>> + port@0: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: DITHER input, usually from a POSTMASK or GAMMA block. >>>>>> + >>>>>> + port@1: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: >>>>>> + DITHER output to the input of the next desired component in the >>>>>> + display pipeline, for example one of the available DSC compressors, >>>>>> + DP_INTF, DSI, LVDS or others. >>>>>> + >>>>>> + required: >>>>>> + - port@0 >>>>>> + - port@1 >>>>>> + >>>>>> required: >>>>>> - compatible >>>>>> - reg >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>> index 3a82aec9021c..b567e3d58aa1 100644 >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>> @@ -71,13 +71,34 @@ properties: >>>>>> Output port node. This port should be connected to the input port of an >>>>>> attached HDMI, LVDS or DisplayPort encoder chip. >>>>>> >>>>>> + ports: >>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>> + >>>>>> + properties: >>>>>> + port@0: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: DPI input port >>>>>> + >>>>>> + port@1: >>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input >>>>> >>>>> This is wrong. The existing 'port' is the output. 'port' and 'port@0' >>>>> are treated as the same thing. Since you are adding an input port, the >>>>> new port has to be 'port@1' (or any number but 0). >>>>> >>>>> I haven't looked at the driver code, but it should request port 0 and >>>>> always get the output port. And requesting port 1 will return an error >>>>> or the input port. >>>> >>>> Hello Rob, >>>> >>>> I want to remind you that in v2 of this series you said that it'd be wrong for >>>> port@0 to be an output, I replied that you misread that as I had modeled it indeed >>>> as an input, and then you gave me your Reviewed-by tag. >>> >>> I have not misread it. Then I guess I forgot about it and missed it the >>> next time on v3. >>> >> >> Okay, that was some misunderstanding then - it's fine, no problem. >> >>>> Anyway - I get your concern about the previous behavior of `port`, but I chose to >>>> model this that way purely for consistency. >>>> >>>> First of all - the driver(s) will check if we're feeding a full graph, as it will >>>> indeed first check if port@1 is present: if it is, then it follows this scheme with >>>> port@0 as INPUT and port@1 as OUTPUT. >>>> If the component in port@0 is an OUTPUT, the bridge attach will fail. >>>> >>>> Getting to bindings themselves, then... it would be a mistake to model port@0 as an >>>> output and port@1 as an input, because that would be not only inconsistent with the >>>> DRM Bridge bindings, but would be highly confusing when reading the devicetree. >>> >>> Somewhat confusing, yes. Highly, no. Put a comment in the DT. >>> >> >> "Somewhat or highly" boils down to personal opinion, so I still go for "highly" >> but won't argue about that as wouldn't be productive and would bring us nowhere >> anyway, so, whatever :-) >> >> Putting a comment in DT is an option, yes, but that comment would need to be put >> on all of the MediaTek SoC device trees - current and future - and IMO would scream >> "inconsistency warning" (in different words, of course) all over, which honestly >> doesn't really look clean... at least to my eyes... > > What I find more confusing is my updated DT doesn't work with my > existing kernel... > >>>> Please note that the bridge bindings are always declaring port@0 as an INPUT and >>>> other ports as OUTPUT(s). >>> >>> There is no guarantee on that. Port numbering is local to the bridge and >>> opaque to anything outside that bridge. That is why you have to document >>> what each port represents. >>> >> >> I know and I agree that there's no guarantee on the numbering. I can see that in >> other non-drm-bridge bindings, and that's fine. >> >>> Would we have followed that convention if all the ports were defined >>> from the start? Certainly. But that didn't happen and you are stuck with >>> the existing binding and ABI. >>> >> >> I thought about adding a new compatible for the new port scheme, but that looked >> even worse to me as, after doing that (yeah, I actually went for it in the first >> version that I have never sent upstream) during my own proof-read I started >> screaming "HACK! HACK! NOOO!" all over, and rewritten the entire thing. >> >>>> As an example, you can check display/bridge/analogix,anx7625.yaml or >>>> display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise >>>> display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml >>>> (and others) for display controllers, which do all conform to this logic, where >>>> the input is always @0, and the output is @1. >>>> >>>> Of course, doing this required me to do extra changes to the MTK DRM drivers to >>>> actually be retro-compatible with the old devicetrees as I explained before. >>> >>> You can't fix existing software... >>> >> >> That's true, but I don't see that as an "excuse" (grant me this term please) to >> "carelessly" keep it in a suboptimal state. >> >> This driver has been growing almost uncontrollably with (wrong, anyway!) >> board-specific component vectors - and writing a new one would just add up >> more code duplication to the mix and/or worsen the maintainability of older >> MediaTek SoCs (as the "old" driver will get forgotten and never updated anymore). >> >>> If you want to break the ABI, then that's ultimately up to you and >>> Mediatek maintainers to decide0. This case is easy to avoid, why would >>> you knowingly break the ABI here. >> >> Because if we don't do this, we condemn (forever) this driver, or part of it >> to have an inverted port scheme compared to either: >> A. The other drm/mediatek components; or >> B. The other bridge drivers (of which, some are used with MTK as well) >> >> ...and we also condemn (forever, again) all MediaTek device trees to scream >> "port inconsistency warning: A for drm/mediatek components, B for every other >> thing", which would scream "drm/mediatek is somewhat broken", which can be >> avoided. > > Sure. The cost is just an ABI break to do that. > >>> OTOH, this seems like a big enough >>> change I would imagine it is a matter of time before supporting a >>> missing OF graph for the components will be a problem. >>> >> >> Sorry I didn't understand this part, can you please-please-please reword? > > I think keeping the kernel working with the old and new binding will > be a challenge. Partly because testing with the old binding won't > happen, but also if the binding and drivers continue to evolve. So > while maintaining old ABI might be possible with this change, it will > continue to be an issue with each change. That's... exactly my point, so I am happy that we agree on that. > BTW, did you actually test > backwards compatibility with this? I can see you fallback to the old > binding, but there's a lot of other changes in there I can't really > tell by looking at it. Short answer: Yes, largely Long answer: Yes, with this series applied, I have tested both *old* and *new* devicetrees on 7 boards with 4 different SoCs and different display pipelines (hence, graphs): one smartphone (MT6795), a "bunch of" Chromebooks (MT8173, MT8186, MT8195), and a SBC (MT8195). Of course those had DSI or eDP displays, with or without DisplayPort external display support. > > What I haven't heard from you is "yes, we need to maintain the ABI" or > "no, we can break it". Decide that, then the path here is simple. No, we have to break it. Cheers, Angelo
On Wed, Oct 16, 2024 at 10:26 AM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 16/10/24 16:00, Rob Herring ha scritto: > > On Wed, Oct 16, 2024 at 4:23 AM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > >> > >> Il 15/10/24 15:48, Rob Herring ha scritto: > >>> On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: > >>>> Il 14/10/24 19:36, Rob Herring ha scritto: > >>>>> On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno > >>>>> <angelogioacchino.delregno@collabora.com> wrote: > >>>>>> > >>>>>> The display IPs in MediaTek SoCs support being interconnected with > >>>>>> different instances of DDP IPs (for example, merge0 or merge1) and/or > >>>>>> with different DDP IPs (for example, rdma can be connected with either > >>>>>> color, dpi, dsi, merge, etc), forming a full Display Data Path that > >>>>>> ends with an actual display. > >>>>>> > >>>>>> The final display pipeline is effectively board specific, as it does > >>>>>> depend on the display that is attached to it, and eventually on the > >>>>>> sensors supported by the board (for example, Adaptive Ambient Light > >>>>>> would need an Ambient Light Sensor, otherwise it's pointless!), other > >>>>>> than the output type. > >>>>>> > >>>>>> Add support for OF graphs to most of the MediaTek DDP (display) bindings > >>>>>> to add flexibility to build custom hardware paths, hence enabling board > >>>>>> specific configuration of the display pipeline and allowing to finally > >>>>>> migrate away from using hardcoded paths. > >>>>>> > >>>>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > >>>>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > >>>>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > >>>>>> Reviewed-by: CK Hu <ck.hu@mediatek.com> > >>>>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > >>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > >>>>>> --- > >>>>>> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ > >>>>>> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ > >>>>>> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- > >>>>>> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ > >>>>>> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- > >>>>>> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ > >>>>>> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ > >>>>>> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ > >>>>>> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ > >>>>>> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ > >>>>>> 16 files changed, 372 insertions(+), 3 deletions(-) > >>>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>>>> index cf24434854ff..47ddba5c41af 100644 > >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml > >>>>>> @@ -62,6 +62,27 @@ properties: > >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>>>> maxItems: 1 > >>>>>> > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + description: > >>>>>> + Input and output ports can have multiple endpoints, each of those > >>>>>> + connects to either the primary, secondary, etc, display pipeline. > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: AAL input port > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: > >>>>>> + AAL output to the next component's input, for example could be one > >>>>>> + of many gamma, overdrive or other blocks. > >>>>>> + > >>>>>> + required: > >>>>>> + - port@0 > >>>>>> + - port@1 > >>>>>> + > >>>>>> required: > >>>>>> - compatible > >>>>>> - reg > >>>>>> @@ -89,5 +110,24 @@ examples: > >>>>>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; > >>>>>> clocks = <&mmsys CLK_MM_DISP_AAL>; > >>>>>> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; > >>>>>> + > >>>>>> + ports { > >>>>>> + #address-cells = <1>; > >>>>>> + #size-cells = <0>; > >>>>>> + > >>>>>> + port@0 { > >>>>>> + reg = <0>; > >>>>>> + aal0_in: endpoint { > >>>>>> + remote-endpoint = <&ccorr0_out>; > >>>>>> + }; > >>>>>> + }; > >>>>>> + > >>>>>> + port@1 { > >>>>>> + reg = <1>; > >>>>>> + aal0_out: endpoint { > >>>>>> + remote-endpoint = <&gamma0_in>; > >>>>>> + }; > >>>>>> + }; > >>>>>> + }; > >>>>>> }; > >>>>>> }; > >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>>>> index 9f8366763831..fca8e7bb0cbc 100644 > >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml > >>>>>> @@ -57,6 +57,27 @@ properties: > >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>>>> maxItems: 1 > >>>>>> > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + description: > >>>>>> + Input and output ports can have multiple endpoints, each of those > >>>>>> + connects to either the primary, secondary, etc, display pipeline. > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: CCORR input port > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: > >>>>>> + CCORR output to the input of the next desired component in the > >>>>>> + display pipeline, usually only one of the available AAL blocks. > >>>>>> + > >>>>>> + required: > >>>>>> + - port@0 > >>>>>> + - port@1 > >>>>>> + > >>>>>> required: > >>>>>> - compatible > >>>>>> - reg > >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>>>> index 7df786bbad20..6160439ce4d7 100644 > >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml > >>>>>> @@ -65,6 +65,28 @@ properties: > >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>>>> maxItems: 1 > >>>>>> > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + description: > >>>>>> + Input and output ports can have multiple endpoints, each of those > >>>>>> + connects to either the primary, secondary, etc, display pipeline. > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: COLOR input port > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: > >>>>>> + COLOR output to the input of the next desired component in the > >>>>>> + display pipeline, for example one of the available CCORR or AAL > >>>>>> + blocks. > >>>>>> + > >>>>>> + required: > >>>>>> + - port@0 > >>>>>> + - port@1 > >>>>>> + > >>>>>> required: > >>>>>> - compatible > >>>>>> - reg > >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>>>> index 6fceb1f95d2a..abaf27916d13 100644 > >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml > >>>>>> @@ -56,6 +56,28 @@ properties: > >>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array > >>>>>> maxItems: 1 > >>>>>> > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + description: > >>>>>> + Input and output ports can have multiple endpoints, each of those > >>>>>> + connects to either the primary, secondary, etc, display pipeline. > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: DITHER input, usually from a POSTMASK or GAMMA block. > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: > >>>>>> + DITHER output to the input of the next desired component in the > >>>>>> + display pipeline, for example one of the available DSC compressors, > >>>>>> + DP_INTF, DSI, LVDS or others. > >>>>>> + > >>>>>> + required: > >>>>>> + - port@0 > >>>>>> + - port@1 > >>>>>> + > >>>>>> required: > >>>>>> - compatible > >>>>>> - reg > >>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>>>> index 3a82aec9021c..b567e3d58aa1 100644 > >>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > >>>>>> @@ -71,13 +71,34 @@ properties: > >>>>>> Output port node. This port should be connected to the input port of an > >>>>>> attached HDMI, LVDS or DisplayPort encoder chip. > >>>>>> > >>>>>> + ports: > >>>>>> + $ref: /schemas/graph.yaml#/properties/ports > >>>>>> + > >>>>>> + properties: > >>>>>> + port@0: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: DPI input port > >>>>>> + > >>>>>> + port@1: > >>>>>> + $ref: /schemas/graph.yaml#/properties/port > >>>>>> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input > >>>>> > >>>>> This is wrong. The existing 'port' is the output. 'port' and 'port@0' > >>>>> are treated as the same thing. Since you are adding an input port, the > >>>>> new port has to be 'port@1' (or any number but 0). > >>>>> > >>>>> I haven't looked at the driver code, but it should request port 0 and > >>>>> always get the output port. And requesting port 1 will return an error > >>>>> or the input port. > >>>> > >>>> Hello Rob, > >>>> > >>>> I want to remind you that in v2 of this series you said that it'd be wrong for > >>>> port@0 to be an output, I replied that you misread that as I had modeled it indeed > >>>> as an input, and then you gave me your Reviewed-by tag. > >>> > >>> I have not misread it. Then I guess I forgot about it and missed it the > >>> next time on v3. > >>> > >> > >> Okay, that was some misunderstanding then - it's fine, no problem. > >> > >>>> Anyway - I get your concern about the previous behavior of `port`, but I chose to > >>>> model this that way purely for consistency. > >>>> > >>>> First of all - the driver(s) will check if we're feeding a full graph, as it will > >>>> indeed first check if port@1 is present: if it is, then it follows this scheme with > >>>> port@0 as INPUT and port@1 as OUTPUT. > >>>> If the component in port@0 is an OUTPUT, the bridge attach will fail. > >>>> > >>>> Getting to bindings themselves, then... it would be a mistake to model port@0 as an > >>>> output and port@1 as an input, because that would be not only inconsistent with the > >>>> DRM Bridge bindings, but would be highly confusing when reading the devicetree. > >>> > >>> Somewhat confusing, yes. Highly, no. Put a comment in the DT. > >>> > >> > >> "Somewhat or highly" boils down to personal opinion, so I still go for "highly" > >> but won't argue about that as wouldn't be productive and would bring us nowhere > >> anyway, so, whatever :-) > >> > >> Putting a comment in DT is an option, yes, but that comment would need to be put > >> on all of the MediaTek SoC device trees - current and future - and IMO would scream > >> "inconsistency warning" (in different words, of course) all over, which honestly > >> doesn't really look clean... at least to my eyes... > > > > What I find more confusing is my updated DT doesn't work with my > > existing kernel... > > > >>>> Please note that the bridge bindings are always declaring port@0 as an INPUT and > >>>> other ports as OUTPUT(s). > >>> > >>> There is no guarantee on that. Port numbering is local to the bridge and > >>> opaque to anything outside that bridge. That is why you have to document > >>> what each port represents. > >>> > >> > >> I know and I agree that there's no guarantee on the numbering. I can see that in > >> other non-drm-bridge bindings, and that's fine. > >> > >>> Would we have followed that convention if all the ports were defined > >>> from the start? Certainly. But that didn't happen and you are stuck with > >>> the existing binding and ABI. > >>> > >> > >> I thought about adding a new compatible for the new port scheme, but that looked > >> even worse to me as, after doing that (yeah, I actually went for it in the first > >> version that I have never sent upstream) during my own proof-read I started > >> screaming "HACK! HACK! NOOO!" all over, and rewritten the entire thing. > >> > >>>> As an example, you can check display/bridge/analogix,anx7625.yaml or > >>>> display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise > >>>> display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml > >>>> (and others) for display controllers, which do all conform to this logic, where > >>>> the input is always @0, and the output is @1. > >>>> > >>>> Of course, doing this required me to do extra changes to the MTK DRM drivers to > >>>> actually be retro-compatible with the old devicetrees as I explained before. > >>> > >>> You can't fix existing software... > >>> > >> > >> That's true, but I don't see that as an "excuse" (grant me this term please) to > >> "carelessly" keep it in a suboptimal state. > >> > >> This driver has been growing almost uncontrollably with (wrong, anyway!) > >> board-specific component vectors - and writing a new one would just add up > >> more code duplication to the mix and/or worsen the maintainability of older > >> MediaTek SoCs (as the "old" driver will get forgotten and never updated anymore). > >> > >>> If you want to break the ABI, then that's ultimately up to you and > >>> Mediatek maintainers to decide0. This case is easy to avoid, why would > >>> you knowingly break the ABI here. > >> > >> Because if we don't do this, we condemn (forever) this driver, or part of it > >> to have an inverted port scheme compared to either: > >> A. The other drm/mediatek components; or > >> B. The other bridge drivers (of which, some are used with MTK as well) > >> > >> ...and we also condemn (forever, again) all MediaTek device trees to scream > >> "port inconsistency warning: A for drm/mediatek components, B for every other > >> thing", which would scream "drm/mediatek is somewhat broken", which can be > >> avoided. > > > > Sure. The cost is just an ABI break to do that. > > > >>> OTOH, this seems like a big enough > >>> change I would imagine it is a matter of time before supporting a > >>> missing OF graph for the components will be a problem. > >>> > >> > >> Sorry I didn't understand this part, can you please-please-please reword? > > > > I think keeping the kernel working with the old and new binding will > > be a challenge. Partly because testing with the old binding won't > > happen, but also if the binding and drivers continue to evolve. So > > while maintaining old ABI might be possible with this change, it will > > continue to be an issue with each change. > > That's... exactly my point, so I am happy that we agree on that. > > > BTW, did you actually test > > backwards compatibility with this? I can see you fallback to the old > > binding, but there's a lot of other changes in there I can't really > > tell by looking at it. > > > Short answer: Yes, largely > > Long answer: > > Yes, with this series applied, I have tested both *old* and *new* devicetrees on > 7 boards with 4 different SoCs and different display pipelines (hence, graphs): > one smartphone (MT6795), a "bunch of" Chromebooks (MT8173, MT8186, MT8195), and > a SBC (MT8195). > > Of course those had DSI or eDP displays, with or without DisplayPort external > display support. > > > > > What I haven't heard from you is "yes, we need to maintain the ABI" or > > "no, we can break it". Decide that, then the path here is simple. > > No, we have to break it. Okay, then my reviewed-by stands. But please make it clear in the binding and dts commit msgs that it is an ABI break. Rob
Il 16/10/24 18:09, Rob Herring ha scritto: > On Wed, Oct 16, 2024 at 10:26 AM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> Il 16/10/24 16:00, Rob Herring ha scritto: >>> On Wed, Oct 16, 2024 at 4:23 AM AngeloGioacchino Del Regno >>> <angelogioacchino.delregno@collabora.com> wrote: >>>> >>>> Il 15/10/24 15:48, Rob Herring ha scritto: >>>>> On Tue, Oct 15, 2024 at 10:32:22AM +0200, AngeloGioacchino Del Regno wrote: >>>>>> Il 14/10/24 19:36, Rob Herring ha scritto: >>>>>>> On Mon, Oct 14, 2024 at 3:51 AM AngeloGioacchino Del Regno >>>>>>> <angelogioacchino.delregno@collabora.com> wrote: >>>>>>>> >>>>>>>> The display IPs in MediaTek SoCs support being interconnected with >>>>>>>> different instances of DDP IPs (for example, merge0 or merge1) and/or >>>>>>>> with different DDP IPs (for example, rdma can be connected with either >>>>>>>> color, dpi, dsi, merge, etc), forming a full Display Data Path that >>>>>>>> ends with an actual display. >>>>>>>> >>>>>>>> The final display pipeline is effectively board specific, as it does >>>>>>>> depend on the display that is attached to it, and eventually on the >>>>>>>> sensors supported by the board (for example, Adaptive Ambient Light >>>>>>>> would need an Ambient Light Sensor, otherwise it's pointless!), other >>>>>>>> than the output type. >>>>>>>> >>>>>>>> Add support for OF graphs to most of the MediaTek DDP (display) bindings >>>>>>>> to add flexibility to build custom hardware paths, hence enabling board >>>>>>>> specific configuration of the display pipeline and allowing to finally >>>>>>>> migrate away from using hardcoded paths. >>>>>>>> >>>>>>>> Reviewed-by: Rob Herring (Arm) <robh@kernel.org> >>>>>>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>>>> Reviewed-by: CK Hu <ck.hu@mediatek.com> >>>>>>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>>>>>> --- >>>>>>>> .../display/mediatek/mediatek,aal.yaml | 40 +++++++++++++++++++ >>>>>>>> .../display/mediatek/mediatek,ccorr.yaml | 21 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,color.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,dither.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,dpi.yaml | 25 +++++++++++- >>>>>>>> .../display/mediatek/mediatek,dsc.yaml | 24 +++++++++++ >>>>>>>> .../display/mediatek/mediatek,dsi.yaml | 27 ++++++++++++- >>>>>>>> .../display/mediatek/mediatek,ethdr.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,gamma.yaml | 19 +++++++++ >>>>>>>> .../display/mediatek/mediatek,merge.yaml | 23 +++++++++++ >>>>>>>> .../display/mediatek/mediatek,od.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,ovl-2l.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,ovl.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,postmask.yaml | 21 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,rdma.yaml | 22 ++++++++++ >>>>>>>> .../display/mediatek/mediatek,ufoe.yaml | 21 ++++++++++ >>>>>>>> 16 files changed, 372 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>>>> index cf24434854ff..47ddba5c41af 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml >>>>>>>> @@ -62,6 +62,27 @@ properties: >>>>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>>>> maxItems: 1 >>>>>>>> >>>>>>>> + ports: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>>>> + description: >>>>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + port@0: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: AAL input port >>>>>>>> + >>>>>>>> + port@1: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: >>>>>>>> + AAL output to the next component's input, for example could be one >>>>>>>> + of many gamma, overdrive or other blocks. >>>>>>>> + >>>>>>>> + required: >>>>>>>> + - port@0 >>>>>>>> + - port@1 >>>>>>>> + >>>>>>>> required: >>>>>>>> - compatible >>>>>>>> - reg >>>>>>>> @@ -89,5 +110,24 @@ examples: >>>>>>>> power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; >>>>>>>> clocks = <&mmsys CLK_MM_DISP_AAL>; >>>>>>>> mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; >>>>>>>> + >>>>>>>> + ports { >>>>>>>> + #address-cells = <1>; >>>>>>>> + #size-cells = <0>; >>>>>>>> + >>>>>>>> + port@0 { >>>>>>>> + reg = <0>; >>>>>>>> + aal0_in: endpoint { >>>>>>>> + remote-endpoint = <&ccorr0_out>; >>>>>>>> + }; >>>>>>>> + }; >>>>>>>> + >>>>>>>> + port@1 { >>>>>>>> + reg = <1>; >>>>>>>> + aal0_out: endpoint { >>>>>>>> + remote-endpoint = <&gamma0_in>; >>>>>>>> + }; >>>>>>>> + }; >>>>>>>> + }; >>>>>>>> }; >>>>>>>> }; >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>>>> index 9f8366763831..fca8e7bb0cbc 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml >>>>>>>> @@ -57,6 +57,27 @@ properties: >>>>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>>>> maxItems: 1 >>>>>>>> >>>>>>>> + ports: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>>>> + description: >>>>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + port@0: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: CCORR input port >>>>>>>> + >>>>>>>> + port@1: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: >>>>>>>> + CCORR output to the input of the next desired component in the >>>>>>>> + display pipeline, usually only one of the available AAL blocks. >>>>>>>> + >>>>>>>> + required: >>>>>>>> + - port@0 >>>>>>>> + - port@1 >>>>>>>> + >>>>>>>> required: >>>>>>>> - compatible >>>>>>>> - reg >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>>>> index 7df786bbad20..6160439ce4d7 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml >>>>>>>> @@ -65,6 +65,28 @@ properties: >>>>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>>>> maxItems: 1 >>>>>>>> >>>>>>>> + ports: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>>>> + description: >>>>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + port@0: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: COLOR input port >>>>>>>> + >>>>>>>> + port@1: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: >>>>>>>> + COLOR output to the input of the next desired component in the >>>>>>>> + display pipeline, for example one of the available CCORR or AAL >>>>>>>> + blocks. >>>>>>>> + >>>>>>>> + required: >>>>>>>> + - port@0 >>>>>>>> + - port@1 >>>>>>>> + >>>>>>>> required: >>>>>>>> - compatible >>>>>>>> - reg >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>>>> index 6fceb1f95d2a..abaf27916d13 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml >>>>>>>> @@ -56,6 +56,28 @@ properties: >>>>>>>> $ref: /schemas/types.yaml#/definitions/phandle-array >>>>>>>> maxItems: 1 >>>>>>>> >>>>>>>> + ports: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>>>> + description: >>>>>>>> + Input and output ports can have multiple endpoints, each of those >>>>>>>> + connects to either the primary, secondary, etc, display pipeline. >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + port@0: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: DITHER input, usually from a POSTMASK or GAMMA block. >>>>>>>> + >>>>>>>> + port@1: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: >>>>>>>> + DITHER output to the input of the next desired component in the >>>>>>>> + display pipeline, for example one of the available DSC compressors, >>>>>>>> + DP_INTF, DSI, LVDS or others. >>>>>>>> + >>>>>>>> + required: >>>>>>>> + - port@0 >>>>>>>> + - port@1 >>>>>>>> + >>>>>>>> required: >>>>>>>> - compatible >>>>>>>> - reg >>>>>>>> diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>>>> index 3a82aec9021c..b567e3d58aa1 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>>>> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml >>>>>>>> @@ -71,13 +71,34 @@ properties: >>>>>>>> Output port node. This port should be connected to the input port of an >>>>>>>> attached HDMI, LVDS or DisplayPort encoder chip. >>>>>>>> >>>>>>>> + ports: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/ports >>>>>>>> + >>>>>>>> + properties: >>>>>>>> + port@0: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: DPI input port >>>>>>>> + >>>>>>>> + port@1: >>>>>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>>>>> + description: DPI output to an HDMI, LVDS or DisplayPort encoder input >>>>>>> >>>>>>> This is wrong. The existing 'port' is the output. 'port' and 'port@0' >>>>>>> are treated as the same thing. Since you are adding an input port, the >>>>>>> new port has to be 'port@1' (or any number but 0). >>>>>>> >>>>>>> I haven't looked at the driver code, but it should request port 0 and >>>>>>> always get the output port. And requesting port 1 will return an error >>>>>>> or the input port. >>>>>> >>>>>> Hello Rob, >>>>>> >>>>>> I want to remind you that in v2 of this series you said that it'd be wrong for >>>>>> port@0 to be an output, I replied that you misread that as I had modeled it indeed >>>>>> as an input, and then you gave me your Reviewed-by tag. >>>>> >>>>> I have not misread it. Then I guess I forgot about it and missed it the >>>>> next time on v3. >>>>> >>>> >>>> Okay, that was some misunderstanding then - it's fine, no problem. >>>> >>>>>> Anyway - I get your concern about the previous behavior of `port`, but I chose to >>>>>> model this that way purely for consistency. >>>>>> >>>>>> First of all - the driver(s) will check if we're feeding a full graph, as it will >>>>>> indeed first check if port@1 is present: if it is, then it follows this scheme with >>>>>> port@0 as INPUT and port@1 as OUTPUT. >>>>>> If the component in port@0 is an OUTPUT, the bridge attach will fail. >>>>>> >>>>>> Getting to bindings themselves, then... it would be a mistake to model port@0 as an >>>>>> output and port@1 as an input, because that would be not only inconsistent with the >>>>>> DRM Bridge bindings, but would be highly confusing when reading the devicetree. >>>>> >>>>> Somewhat confusing, yes. Highly, no. Put a comment in the DT. >>>>> >>>> >>>> "Somewhat or highly" boils down to personal opinion, so I still go for "highly" >>>> but won't argue about that as wouldn't be productive and would bring us nowhere >>>> anyway, so, whatever :-) >>>> >>>> Putting a comment in DT is an option, yes, but that comment would need to be put >>>> on all of the MediaTek SoC device trees - current and future - and IMO would scream >>>> "inconsistency warning" (in different words, of course) all over, which honestly >>>> doesn't really look clean... at least to my eyes... >>> >>> What I find more confusing is my updated DT doesn't work with my >>> existing kernel... >>> >>>>>> Please note that the bridge bindings are always declaring port@0 as an INPUT and >>>>>> other ports as OUTPUT(s). >>>>> >>>>> There is no guarantee on that. Port numbering is local to the bridge and >>>>> opaque to anything outside that bridge. That is why you have to document >>>>> what each port represents. >>>>> >>>> >>>> I know and I agree that there's no guarantee on the numbering. I can see that in >>>> other non-drm-bridge bindings, and that's fine. >>>> >>>>> Would we have followed that convention if all the ports were defined >>>>> from the start? Certainly. But that didn't happen and you are stuck with >>>>> the existing binding and ABI. >>>>> >>>> >>>> I thought about adding a new compatible for the new port scheme, but that looked >>>> even worse to me as, after doing that (yeah, I actually went for it in the first >>>> version that I have never sent upstream) during my own proof-read I started >>>> screaming "HACK! HACK! NOOO!" all over, and rewritten the entire thing. >>>> >>>>>> As an example, you can check display/bridge/analogix,anx7625.yaml or >>>>>> display/bridge/samsung,mipi-dsim.yaml (and others) for bridges, otherwise >>>>>> display/st,stm32mp25-lvds.yaml or display/allwinner,sun4i-a10-display-frontend.yaml >>>>>> (and others) for display controllers, which do all conform to this logic, where >>>>>> the input is always @0, and the output is @1. >>>>>> >>>>>> Of course, doing this required me to do extra changes to the MTK DRM drivers to >>>>>> actually be retro-compatible with the old devicetrees as I explained before. >>>>> >>>>> You can't fix existing software... >>>>> >>>> >>>> That's true, but I don't see that as an "excuse" (grant me this term please) to >>>> "carelessly" keep it in a suboptimal state. >>>> >>>> This driver has been growing almost uncontrollably with (wrong, anyway!) >>>> board-specific component vectors - and writing a new one would just add up >>>> more code duplication to the mix and/or worsen the maintainability of older >>>> MediaTek SoCs (as the "old" driver will get forgotten and never updated anymore). >>>> >>>>> If you want to break the ABI, then that's ultimately up to you and >>>>> Mediatek maintainers to decide0. This case is easy to avoid, why would >>>>> you knowingly break the ABI here. >>>> >>>> Because if we don't do this, we condemn (forever) this driver, or part of it >>>> to have an inverted port scheme compared to either: >>>> A. The other drm/mediatek components; or >>>> B. The other bridge drivers (of which, some are used with MTK as well) >>>> >>>> ...and we also condemn (forever, again) all MediaTek device trees to scream >>>> "port inconsistency warning: A for drm/mediatek components, B for every other >>>> thing", which would scream "drm/mediatek is somewhat broken", which can be >>>> avoided. >>> >>> Sure. The cost is just an ABI break to do that. >>> >>>>> OTOH, this seems like a big enough >>>>> change I would imagine it is a matter of time before supporting a >>>>> missing OF graph for the components will be a problem. >>>>> >>>> >>>> Sorry I didn't understand this part, can you please-please-please reword? >>> >>> I think keeping the kernel working with the old and new binding will >>> be a challenge. Partly because testing with the old binding won't >>> happen, but also if the binding and drivers continue to evolve. So >>> while maintaining old ABI might be possible with this change, it will >>> continue to be an issue with each change. >> >> That's... exactly my point, so I am happy that we agree on that. >> >>> BTW, did you actually test >>> backwards compatibility with this? I can see you fallback to the old >>> binding, but there's a lot of other changes in there I can't really >>> tell by looking at it. >> >> >> Short answer: Yes, largely >> >> Long answer: >> >> Yes, with this series applied, I have tested both *old* and *new* devicetrees on >> 7 boards with 4 different SoCs and different display pipelines (hence, graphs): >> one smartphone (MT6795), a "bunch of" Chromebooks (MT8173, MT8186, MT8195), and >> a SBC (MT8195). >> >> Of course those had DSI or eDP displays, with or without DisplayPort external >> display support. >> >>> >>> What I haven't heard from you is "yes, we need to maintain the ABI" or >>> "no, we can break it". Decide that, then the path here is simple. >> >> No, we have to break it. > > Okay, then my reviewed-by stands. But please make it clear in the > binding and dts commit msgs that it is an ABI break. > Thanks for that. I'll send a v13 that adds a snippet of text in this commit's description stating that it breaks the ABI, and I will keep your R-b tag. Cheers, Angelo
diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml index cf24434854ff..47ddba5c41af 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,aal.yaml @@ -62,6 +62,27 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: AAL input port + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + AAL output to the next component's input, for example could be one + of many gamma, overdrive or other blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg @@ -89,5 +110,24 @@ examples: power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; clocks = <&mmsys CLK_MM_DISP_AAL>; mediatek,gce-client-reg = <&gce SUBSYS_1401XXXX 0x5000 0x1000>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + aal0_in: endpoint { + remote-endpoint = <&ccorr0_out>; + }; + }; + + port@1 { + reg = <1>; + aal0_out: endpoint { + remote-endpoint = <&gamma0_in>; + }; + }; + }; }; }; diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml index 9f8366763831..fca8e7bb0cbc 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ccorr.yaml @@ -57,6 +57,27 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: CCORR input port + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + CCORR output to the input of the next desired component in the + display pipeline, usually only one of the available AAL blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml index 7df786bbad20..6160439ce4d7 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,color.yaml @@ -65,6 +65,28 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: COLOR input port + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + COLOR output to the input of the next desired component in the + display pipeline, for example one of the available CCORR or AAL + blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml index 6fceb1f95d2a..abaf27916d13 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dither.yaml @@ -56,6 +56,28 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: DITHER input, usually from a POSTMASK or GAMMA block. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + DITHER output to the input of the next desired component in the + display pipeline, for example one of the available DSC compressors, + DP_INTF, DSI, LVDS or others. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml index 3a82aec9021c..b567e3d58aa1 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml @@ -71,13 +71,34 @@ properties: Output port node. This port should be connected to the input port of an attached HDMI, LVDS or DisplayPort encoder chip. + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: DPI input port + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: DPI output to an HDMI, LVDS or DisplayPort encoder input + + required: + - port@0 + - port@1 + required: - compatible - reg - interrupts - clocks - clock-names - - port + +oneOf: + - required: + - port + - required: + - ports allOf: - if: @@ -100,7 +121,7 @@ examples: #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/clock/mt8173-clk.h> - dpi0: dpi@1401d000 { + dpi: dpi@1401d000 { compatible = "mediatek,mt8173-dpi"; reg = <0x1401d000 0x1000>; interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_LOW>; diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml index 2cbdd9ee449d..846de6c17d93 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsc.yaml @@ -49,6 +49,30 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + Display Stream Compression input, usually from one of the DITHER + or MERGE blocks. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + Display Stream Compression output to the input of the next desired + component in the display pipeline, for example to MERGE, DP_INTF, + DPI or DSI. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml index a7aa8fcb0dd1..27ffbccc2a08 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dsi.yaml @@ -77,6 +77,26 @@ properties: Output port node. This port should be connected to the input port of an attached DSI panel or DSI-to-eDP encoder chip. + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input ports can have multiple endpoints, each of those connects + to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: DSI input port, usually from DITHER, DSC or MERGE + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + DSI output to an attached DSI panel, or a DSI-to-X encoder chip + + required: + - port@0 + - port@1 + required: - compatible - reg @@ -86,7 +106,12 @@ required: - clock-names - phys - phy-names - - port + +oneOf: + - required: + - port + - required: + - ports unevaluatedProperties: false diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml index 677882348ede..98db47894eeb 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ethdr.yaml @@ -110,6 +110,28 @@ properties: include/dt-bindings/gce/<chip>-gce.h, mapping to the register of display function block. + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: ETHDR input, usually from one of the MERGE blocks. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + ETHDR output to the input of the next desired component in the + display pipeline, for example one of the available MERGE blocks, + or others. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml index 6823d3ce5049..48542dc7e784 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,gamma.yaml @@ -65,6 +65,25 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: GAMMA input, usually from one of the AAL blocks. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + GAMMA output to the input of the next desired component in the + display pipeline, for example one of the available DITHER or + POSTMASK blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml index dae839279950..0de9f64f3f84 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,merge.yaml @@ -77,6 +77,29 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: + MERGE input port, usually from DITHER, DPI, DSC, DSI, MDP_RDMA, + ETHDR or even from a different MERGE block + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + MERGE output to a DSC, DPI, DP_INTF, DSI, ETHDR, Write DMA, or + a different MERGE block, or others. + + required: + - port@0 + - port@1 + resets: description: reset controller See Documentation/devicetree/bindings/reset/reset.txt for details. diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml index 831c653caffd..71534febd49c 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,od.yaml @@ -38,6 +38,28 @@ properties: items: - description: OD Clock + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: OD input port, usually from an AAL block + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + OD output to the input of the next desired component in the + display pipeline, for example one of the available RDMA or + other blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml index c7dd0ef02dcf..bacdfe7d08a6 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl-2l.yaml @@ -57,6 +57,28 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: OVL input port from MMSYS, VDOSYS or other OVLs + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + OVL output to the input of the next desired component in the + display pipeline, for example one of the available COLOR, RDMA + or WDMA blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml index d55611c7ce5e..9ea796a033b2 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ovl.yaml @@ -75,6 +75,28 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: OVL input port from MMSYS or one of multiple VDOSYS + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + OVL output to the input of the next desired component in the + display pipeline, for example one of the available COLOR, RDMA + or WDMA blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml index 11fe32e50a59..fb6fe4742624 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,postmask.yaml @@ -52,6 +52,27 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: POSTMASK input port, usually from GAMMA + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + POSTMASK output to the input of the next desired component in the + display pipeline, for example one of the available DITHER blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml index 4cadb245d028..878f676b581f 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,rdma.yaml @@ -87,6 +87,28 @@ properties: $ref: /schemas/types.yaml#/definitions/phandle-array maxItems: 1 + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: RDMA input port, usually from MMSYS, OD or OVL + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + RDMA output to the input of the next desired component in the + display pipeline, for example one of the available COLOR, DPI, + DSI, MERGE or UFOE blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg diff --git a/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml b/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml index 39e3e2d4a0db..61a5e22effbf 100644 --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,ufoe.yaml @@ -43,6 +43,27 @@ properties: items: - description: UFOe Clock + ports: + $ref: /schemas/graph.yaml#/properties/ports + description: + Input and output ports can have multiple endpoints, each of those + connects to either the primary, secondary, etc, display pipeline. + + properties: + port@0: + $ref: /schemas/graph.yaml#/properties/port + description: UFOE input, usually from one of the RDMA blocks. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: + UFOE output to the input of the next desired component in the + display pipeline, usually one of the available DSI blocks. + + required: + - port@0 + - port@1 + required: - compatible - reg