* [PATCH v10 1/3] dt-bindings: display: mediatek: Add OF graph support for board path [not found] <20240910105054.125005-1-angelogioacchino.delregno@collabora.com> @ 2024-09-10 10:51 ` AngeloGioacchino Del Regno 2024-09-10 10:51 ` [PATCH v10 2/3] dt-bindings: arm: mediatek: mmsys: " AngeloGioacchino Del Regno 2024-09-10 10:51 ` [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths AngeloGioacchino Del Regno 2 siblings, 0 replies; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-09-10 10:51 UTC (permalink / raw) To: chunkuang.hu Cc: robh, krzysztof.kozlowski+dt, conor+dt, p.zabel, airlied, daniel, maarten.lankhorst, mripard, tzimmermann, matthias.bgg, angelogioacchino.delregno, shawn.sung, yu-chang.lee, ck.hu, jitao.shi, devicetree, linux-kernel, dri-devel, linux-mediatek, linux-arm-kernel, wenst, kernel, sui.jingfeng, michael, sjoerd, Alexandre Mergnat, Michael Walle 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 + + 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 -- 2.46.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v10 2/3] dt-bindings: arm: mediatek: mmsys: Add OF graph support for board path [not found] <20240910105054.125005-1-angelogioacchino.delregno@collabora.com> 2024-09-10 10:51 ` [PATCH v10 1/3] dt-bindings: display: mediatek: Add OF graph support for board path AngeloGioacchino Del Regno @ 2024-09-10 10:51 ` AngeloGioacchino Del Regno 2024-09-10 10:51 ` [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths AngeloGioacchino Del Regno 2 siblings, 0 replies; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-09-10 10:51 UTC (permalink / raw) To: chunkuang.hu Cc: robh, krzysztof.kozlowski+dt, conor+dt, p.zabel, airlied, daniel, maarten.lankhorst, mripard, tzimmermann, matthias.bgg, angelogioacchino.delregno, shawn.sung, yu-chang.lee, ck.hu, jitao.shi, devicetree, linux-kernel, dri-devel, linux-mediatek, linux-arm-kernel, wenst, kernel, sui.jingfeng, michael, sjoerd, Alexandre Mergnat, Michael Walle Document OF graph on MMSYS/VDOSYS: this supports up to three DDP paths per HW instance (so potentially up to six displays for multi-vdo SoCs). The MMSYS or VDOSYS is always the first component in the DDP pipeline, so it only supports an output port with multiple endpoints - where each endpoint defines the starting point for one of the (currently three) possible hardware paths. Reviewed-by: Rob Herring (Arm) <robh@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> --- .../bindings/arm/mediatek/mediatek,mmsys.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml index b3c6888c1457..3f4262e93c78 100644 --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml @@ -93,6 +93,34 @@ properties: '#reset-cells': const: 1 + port: + $ref: /schemas/graph.yaml#/properties/port + description: + Output port node. This port connects the MMSYS/VDOSYS output to + the first component of one display pipeline, for example one of + the available OVL or RDMA blocks. + Some MediaTek SoCs support multiple display outputs per MMSYS. + properties: + endpoint@0: + $ref: /schemas/graph.yaml#/properties/endpoint + description: Output to the primary display pipeline + + endpoint@1: + $ref: /schemas/graph.yaml#/properties/endpoint + description: Output to the secondary display pipeline + + endpoint@2: + $ref: /schemas/graph.yaml#/properties/endpoint + description: Output to the tertiary display pipeline + + anyOf: + - required: + - endpoint@0 + - required: + - endpoint@1 + - required: + - endpoint@2 + required: - compatible - reg -- 2.46.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths [not found] <20240910105054.125005-1-angelogioacchino.delregno@collabora.com> 2024-09-10 10:51 ` [PATCH v10 1/3] dt-bindings: display: mediatek: Add OF graph support for board path AngeloGioacchino Del Regno 2024-09-10 10:51 ` [PATCH v10 2/3] dt-bindings: arm: mediatek: mmsys: " AngeloGioacchino Del Regno @ 2024-09-10 10:51 ` AngeloGioacchino Del Regno 2024-10-01 10:07 ` CK Hu (胡俊光) 2 siblings, 1 reply; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-09-10 10:51 UTC (permalink / raw) To: chunkuang.hu Cc: robh, krzysztof.kozlowski+dt, conor+dt, p.zabel, airlied, daniel, maarten.lankhorst, mripard, tzimmermann, matthias.bgg, angelogioacchino.delregno, shawn.sung, yu-chang.lee, ck.hu, jitao.shi, devicetree, linux-kernel, dri-devel, linux-mediatek, linux-arm-kernel, wenst, kernel, sui.jingfeng, michael, sjoerd, Alexandre Mergnat, Michael Walle It is impossible to add each and every possible DDP path combination for each and every possible combination of SoC and board: right now, this driver hardcodes configuration for 10 SoCs and this is going to grow larger and larger, and with new hacks like the introduction of mtk_drm_route which is anyway not enough for all final routes as the DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling DSC preventively doesn't work if the display doesn't support it, or others. Since practically all display IPs in MediaTek SoCs support being interconnected with different instances of other, or the same, IPs or with different IPs and in different combinations, the final DDP pipeline is effectively a board specific configuration. Implement OF graphs support to the mediatek-drm drivers, allowing to stop hardcoding the paths, and preventing this driver to get a huge amount of arrays for each board and SoC combination, also paving the way to share the same mtk_mmsys_driver_data between multiple SoCs, making it more straightforward to add support for new chips. Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- drivers/gpu/drm/mediatek/mtk_disp_drv.h | 1 + .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 40 ++- drivers/gpu/drm/mediatek/mtk_dpi.c | 21 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 253 +++++++++++++++++- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 2 +- drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +- 6 files changed, 309 insertions(+), 22 deletions(-) diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h b/drivers/gpu/drm/mediatek/mtk_disp_drv.h index 082ac18fe04a..94843974851f 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h @@ -108,6 +108,7 @@ size_t mtk_ovl_get_num_formats(struct device *dev); void mtk_ovl_adaptor_add_comp(struct device *dev, struct mtk_mutex *mutex); void mtk_ovl_adaptor_remove_comp(struct device *dev, struct mtk_mutex *mutex); +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node); void mtk_ovl_adaptor_connect(struct device *dev, struct device *mmsys_dev, unsigned int next); void mtk_ovl_adaptor_disconnect(struct device *dev, struct device *mmsys_dev, diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c index c6768210b08b..e9dc89764495 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c @@ -490,6 +490,38 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == data; } +static int ovl_adaptor_of_get_ddp_comp_type(struct device_node *node, + enum mtk_ovl_adaptor_comp_type *ctype) +{ + const struct of_device_id *of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node); + + if (!of_id) + return -EINVAL; + + *ctype = (enum mtk_ovl_adaptor_comp_type)((uintptr_t)of_id->data); + + return 0; +} + +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) +{ + enum mtk_ovl_adaptor_comp_type type; + int ret; + + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); + if (ret) + return false; + + if (type >= OVL_ADAPTOR_TYPE_NUM) + return false; + + /* + * ETHDR and Padding are used exclusively in OVL Adaptor: if this + * component is not one of those, it's likely not an OVL Adaptor path. + */ + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; +} + static int ovl_adaptor_comp_init(struct device *dev, struct component_match **match) { struct mtk_disp_ovl_adaptor *priv = dev_get_drvdata(dev); @@ -499,12 +531,11 @@ static int ovl_adaptor_comp_init(struct device *dev, struct component_match **ma parent = dev->parent->parent->of_node->parent; for_each_child_of_node_scoped(parent, node) { - const struct of_device_id *of_id; enum mtk_ovl_adaptor_comp_type type; - int id; + int id, ret; - of_id = of_match_node(mtk_ovl_adaptor_comp_dt_ids, node); - if (!of_id) + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); + if (ret) continue; if (!of_device_is_available(node)) { @@ -513,7 +544,6 @@ static int ovl_adaptor_comp_init(struct device *dev, struct component_match **ma continue; } - type = (enum mtk_ovl_adaptor_comp_type)(uintptr_t)of_id->data; id = ovl_adaptor_comp_get_id(dev, node, type); if (id < 0) { dev_warn(dev, "Skipping unknown component %pOF\n", diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c b/drivers/gpu/drm/mediatek/mtk_dpi.c index a08d20654954..20a9d589fd75 100644 --- a/drivers/gpu/drm/mediatek/mtk_dpi.c +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c @@ -704,6 +704,20 @@ static int mtk_dpi_bridge_attach(struct drm_bridge *bridge, enum drm_bridge_attach_flags flags) { struct mtk_dpi *dpi = bridge_to_dpi(bridge); + int ret; + + dpi->next_bridge = devm_drm_of_get_bridge(dpi->dev, dpi->dev->of_node, 1, -1); + if (IS_ERR(dpi->next_bridge)) { + ret = PTR_ERR(dpi->next_bridge); + if (ret == -EPROBE_DEFER) + return ret; + + /* Old devicetree has only one endpoint */ + dpi->next_bridge = devm_drm_of_get_bridge(dpi->dev, dpi->dev->of_node, 0, 0); + if (IS_ERR(dpi->next_bridge)) + return dev_err_probe(dpi->dev, PTR_ERR(dpi->next_bridge), + "Failed to get bridge\n"); + } return drm_bridge_attach(bridge->encoder, dpi->next_bridge, &dpi->bridge, flags); @@ -1058,13 +1072,6 @@ static int mtk_dpi_probe(struct platform_device *pdev) if (dpi->irq < 0) return dpi->irq; - dpi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); - if (IS_ERR(dpi->next_bridge)) - return dev_err_probe(dev, PTR_ERR(dpi->next_bridge), - "Failed to get bridge\n"); - - dev_info(dev, "Found bridge node: %pOF\n", dpi->next_bridge->of_node); - platform_set_drvdata(pdev, dpi); dpi->bridge.funcs = &mtk_dpi_bridge_funcs; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 3e807195a0d0..6cff020a1bf8 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -26,6 +26,7 @@ #include "mtk_crtc.h" #include "mtk_ddp_comp.h" +#include "mtk_disp_drv.h" #include "mtk_drm_drv.h" #include "mtk_gem.h" @@ -818,12 +819,235 @@ static const struct of_device_id mtk_ddp_comp_dt_ids[] = { { } }; +static int mtk_drm_of_get_ddp_comp_type(struct device_node *node, enum mtk_ddp_comp_type *ctype) +{ + const struct of_device_id *of_id = of_match_node(mtk_ddp_comp_dt_ids, node); + + if (!of_id) + return -EINVAL; + + *ctype = (enum mtk_ddp_comp_type)((uintptr_t)of_id->data); + + return 0; +} + +static int mtk_drm_of_get_ddp_ep_cid(struct device_node *node, + int output_port, enum mtk_crtc_path crtc_path, + struct device_node **next, unsigned int *cid) +{ + struct device_node *ep_dev_node, *ep_out; + enum mtk_ddp_comp_type comp_type; + int ret; + + ep_out = of_graph_get_endpoint_by_regs(node, output_port, crtc_path); + if (!ep_out) + return -ENOENT; + + ep_dev_node = of_graph_get_remote_port_parent(ep_out); + of_node_put(ep_out); + if (!ep_dev_node) + return -EINVAL; + + /* + * Pass the next node pointer regardless of failures in the later code + * so that if this function is called in a loop it will walk through all + * of the subsequent endpoints anyway. + */ + *next = ep_dev_node; + + if (!of_device_is_available(ep_dev_node)) + return -ENODEV; + + ret = mtk_drm_of_get_ddp_comp_type(ep_dev_node, &comp_type); + if (ret) { + if (mtk_ovl_adaptor_is_comp_present(ep_dev_node)) { + *cid = (unsigned int)DDP_COMPONENT_DRM_OVL_ADAPTOR; + return 0; + } + return ret; + } + + ret = mtk_ddp_comp_get_id(ep_dev_node, comp_type); + if (ret < 0) + return ret; + + /* All ok! Pass the Component ID to the caller. */ + *cid = (unsigned int)ret; + + return 0; +} + +/** + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path + * @dev: The mediatek-drm device + * @cpath: CRTC Path relative to a VDO or MMSYS + * @out_path: Pointer to an array that will contain the new pipeline + * @out_path_len: Number of entries in the pipeline array + * + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending + * on the board-specific desired display configuration; this function walks + * through all of the output endpoints starting from a VDO or MMSYS hardware + * instance and builds the right pipeline as specified in device trees. + * + * Return: + * * %0 - Display HW Pipeline successfully built and validated + * * %-ENOENT - Display pipeline was not specified in device tree + * * %-EINVAL - Display pipeline built but validation failed + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller + */ +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, + const unsigned int **out_path, + unsigned int *out_path_len) +{ + struct device_node *next, *prev, *vdo = dev->parent->of_node; + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; + unsigned int *final_ddp_path; + unsigned short int idx = 0; + bool ovl_adaptor_comp_added = false; + int ret; + + /* Get the first entry for the temp_path array */ + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); + if (ret) { + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); + ovl_adaptor_comp_added = true; + } else { + if (next) + dev_err(dev, "Invalid component %pOF\n", next); + else + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); + + return ret; + } + } + idx++; + + /* + * Walk through port outputs until we reach the last valid mediatek-drm component. + * To be valid, this must end with an "invalid" component that is a display node. + */ + do { + prev = next; + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); + of_node_put(prev); + if (ret) { + of_node_put(next); + break; + } + + /* + * If this is an OVL adaptor exclusive component and one of those + * was already added, don't add another instance of the generic + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether + * to probe that component master driver of which only one instance + * is needed and possible. + */ + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { + if (!ovl_adaptor_comp_added) + ovl_adaptor_comp_added = true; + else + idx--; + } + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); + + /* + * The device component might not be enabled: in that case, don't + * check the last entry and just report that the device is missing. + */ + if (ret == -ENODEV) + return ret; + + /* If the last entry is not a final display output, the configuration is wrong */ + switch (temp_path[idx - 1]) { + case DDP_COMPONENT_DP_INTF0: + case DDP_COMPONENT_DP_INTF1: + case DDP_COMPONENT_DPI0: + case DDP_COMPONENT_DPI1: + case DDP_COMPONENT_DSI0: + case DDP_COMPONENT_DSI1: + case DDP_COMPONENT_DSI2: + case DDP_COMPONENT_DSI3: + break; + default: + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", + temp_path[idx - 1], ret); + return -EINVAL; + } + + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); + if (!final_ddp_path) + return -ENOMEM; + + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); + + /* Pipeline built! */ + *out_path = final_ddp_path; + *out_path_len = idx; + + return 0; +} + +static int mtk_drm_of_ddp_path_build(struct device *dev, struct device_node *node, + struct mtk_mmsys_driver_data *data) +{ + struct device_node *ep_node; + struct of_endpoint of_ep; + bool output_present[MAX_CRTC] = { false }; + int ret; + + for_each_endpoint_of_node(node, ep_node) { + ret = of_graph_parse_endpoint(ep_node, &of_ep); + if (ret) { + dev_err_probe(dev, ret, "Cannot parse endpoint\n"); + break; + } + + if (of_ep.id >= MAX_CRTC) { + ret = dev_err_probe(dev, -EINVAL, + "Invalid endpoint%u number\n", of_ep.port); + break; + } + + output_present[of_ep.id] = true; + } + + if (ret) { + of_node_put(ep_node); + return ret; + } + + if (output_present[CRTC_MAIN]) { + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_MAIN, + &data->main_path, &data->main_len); + if (ret && ret != -ENODEV) + return ret; + } + + if (output_present[CRTC_EXT]) { + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_EXT, + &data->ext_path, &data->ext_len); + if (ret && ret != -ENODEV) + return ret; + } + + if (output_present[CRTC_THIRD]) { + ret = mtk_drm_of_ddp_path_build_one(dev, CRTC_THIRD, + &data->third_path, &data->third_len); + if (ret && ret != -ENODEV) + return ret; + } + + return 0; +} + static int mtk_drm_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct device_node *phandle = dev->parent->of_node; const struct of_device_id *of_id; struct mtk_drm_private *private; + struct mtk_mmsys_driver_data *mtk_drm_data; struct device_node *node; struct component_match *match = NULL; struct platform_device *ovl_adaptor; @@ -844,7 +1068,27 @@ static int mtk_drm_probe(struct platform_device *pdev) if (!of_id) return -ENODEV; - private->data = of_id->data; + mtk_drm_data = (struct mtk_mmsys_driver_data *)of_id->data; + if (!mtk_drm_data) + return -EINVAL; + + /* Try to build the display pipeline from devicetree graphs */ + if (of_graph_is_present(phandle)) { + dev_dbg(dev, "Building display pipeline for MMSYS %u\n", + mtk_drm_data->mmsys_id); + private->data = devm_kmemdup(dev, mtk_drm_data, + sizeof(*mtk_drm_data), GFP_KERNEL); + if (!private->data) + return -ENOMEM; + + ret = mtk_drm_of_ddp_path_build(dev, phandle, private->data); + if (ret) + return ret; + } else { + /* No devicetree graphs support: go with hardcoded paths if present */ + dev_dbg(dev, "Using hardcoded paths for MMSYS %u\n", mtk_drm_data->mmsys_id); + private->data = mtk_drm_data; + }; private->all_drm_private = devm_kmalloc_array(dev, private->data->mmsys_dev_num, sizeof(*private->all_drm_private), @@ -866,12 +1110,11 @@ static int mtk_drm_probe(struct platform_device *pdev) /* Iterate over sibling DISP function blocks */ for_each_child_of_node(phandle->parent, node) { - const struct of_device_id *of_id; enum mtk_ddp_comp_type comp_type; int comp_id; - of_id = of_match_node(mtk_ddp_comp_dt_ids, node); - if (!of_id) + ret = mtk_drm_of_get_ddp_comp_type(node, &comp_type); + if (ret) continue; if (!of_device_is_available(node)) { @@ -880,8 +1123,6 @@ static int mtk_drm_probe(struct platform_device *pdev) continue; } - comp_type = (enum mtk_ddp_comp_type)(uintptr_t)of_id->data; - if (comp_type == MTK_DISP_MUTEX) { int id; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index ce897984de51..675cdc90a440 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -63,7 +63,7 @@ struct mtk_drm_private { struct device *mmsys_dev; struct device_node *comp_node[DDP_COMPONENT_DRM_ID_MAX]; struct mtk_ddp_comp ddp_comp[DDP_COMPONENT_DRM_ID_MAX]; - const struct mtk_mmsys_driver_data *data; + struct mtk_mmsys_driver_data *data; struct drm_atomic_state *suspend_state; unsigned int mbox_index; struct mtk_drm_private **all_drm_private; diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediatek/mtk_dsi.c index eeec641cab60..33ceeb8d6925 100644 --- a/drivers/gpu/drm/mediatek/mtk_dsi.c +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c @@ -988,9 +988,17 @@ static int mtk_dsi_host_attach(struct mipi_dsi_host *host, dsi->lanes = device->lanes; dsi->format = device->format; dsi->mode_flags = device->mode_flags; - dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); - if (IS_ERR(dsi->next_bridge)) - return PTR_ERR(dsi->next_bridge); + dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 1, 0); + if (IS_ERR(dsi->next_bridge)) { + ret = PTR_ERR(dsi->next_bridge); + if (ret == -EPROBE_DEFER) + return ret; + + /* Old devicetree has only one endpoint */ + dsi->next_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 0, 0); + if (IS_ERR(dsi->next_bridge)) + return PTR_ERR(dsi->next_bridge); + } drm_bridge_add(&dsi->bridge); -- 2.46.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-09-10 10:51 ` [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths AngeloGioacchino Del Regno @ 2024-10-01 10:07 ` CK Hu (胡俊光) 2024-10-01 11:33 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 9+ messages in thread From: CK Hu (胡俊光) @ 2024-10-01 10:07 UTC (permalink / raw) To: AngeloGioacchino Del Regno, chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Hi, Angelo: On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: > It is impossible to add each and every possible DDP path combination > for each and every possible combination of SoC and board: right now, > this driver hardcodes configuration for 10 SoCs and this is going to > grow larger and larger, and with new hacks like the introduction of > mtk_drm_route which is anyway not enough for all final routes as the > DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling > DSC preventively doesn't work if the display doesn't support it, or > others. > > Since practically all display IPs in MediaTek SoCs support being > interconnected with different instances of other, or the same, IPs > or with different IPs and in different combinations, the final DDP > pipeline is effectively a board specific configuration. > > Implement OF graphs support to the mediatek-drm drivers, allowing to > stop hardcoding the paths, and preventing this driver to get a huge > amount of arrays for each board and SoC combination, also paving the > way to share the same mtk_mmsys_driver_data between multiple SoCs, > making it more straightforward to add support for new chips. > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- [snip] > + > +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) > +{ > + enum mtk_ovl_adaptor_comp_type type; > + int ret; > + > + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); > + if (ret) > + return false; > + > + if (type >= OVL_ADAPTOR_TYPE_NUM) > + return false; > + > + /* > + * ETHDR and Padding are used exclusively in OVL Adaptor: if this > + * component is not one of those, it's likely not an OVL Adaptor path. > + */ I don't know your logic here. The OVL Adaptor pipeline is: mdp_rdma -> padding ---+ +-------+ Merge -> | | mdp_rdma -> padding ---+ | | | | mdp_rdma -> padding ---+ | | Merge -> | | mdp_rdma -> padding ---+ | | | ETHDR | mdp_rdma -> padding ---+ | | Merge -> | | mdp_rdma -> padding ---+ | | | | mdp_rdma -> padding ---+ | | Merge -> | | mdp_rdma -> padding ---+ +-------+ So mdp_rdma and merge is not OVL Adaptor? > + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; > +} > + > [snip] > + > +/** > + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path > + * @dev: The mediatek-drm device > + * @cpath: CRTC Path relative to a VDO or MMSYS > + * @out_path: Pointer to an array that will contain the new pipeline > + * @out_path_len: Number of entries in the pipeline array > + * > + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending > + * on the board-specific desired display configuration; this function walks > + * through all of the output endpoints starting from a VDO or MMSYS hardware > + * instance and builds the right pipeline as specified in device trees. > + * > + * Return: > + * * %0 - Display HW Pipeline successfully built and validated > + * * %-ENOENT - Display pipeline was not specified in device tree > + * * %-EINVAL - Display pipeline built but validation failed > + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller > + */ > +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, > + const unsigned int **out_path, > + unsigned int *out_path_len) > +{ > + struct device_node *next, *prev, *vdo = dev->parent->of_node; > + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; > + unsigned int *final_ddp_path; > + unsigned short int idx = 0; > + bool ovl_adaptor_comp_added = false; > + int ret; > + > + /* Get the first entry for the temp_path array */ > + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); > + if (ret) { > + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); > + ovl_adaptor_comp_added = true; > + } else { > + if (next) > + dev_err(dev, "Invalid component %pOF\n", next); > + else > + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); > + > + return ret; > + } > + } > + idx++; > + > + /* > + * Walk through port outputs until we reach the last valid mediatek-drm component. > + * To be valid, this must end with an "invalid" component that is a display node. > + */ > + do { > + prev = next; > + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); > + of_node_put(prev); > + if (ret) { > + of_node_put(next); > + break; > + } > + > + /* > + * If this is an OVL adaptor exclusive component and one of those > + * was already added, don't add another instance of the generic > + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether > + * to probe that component master driver of which only one instance > + * is needed and possible. > + */ > + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. Finally, the path would be: mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... Regards, CK > + if (!ovl_adaptor_comp_added) > + ovl_adaptor_comp_added = true; > + else > + idx--; > + } > + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); > + > + /* > + * The device component might not be enabled: in that case, don't > + * check the last entry and just report that the device is missing. > + */ > + if (ret == -ENODEV) > + return ret; > + > + /* If the last entry is not a final display output, the configuration is wrong */ > + switch (temp_path[idx - 1]) { > + case DDP_COMPONENT_DP_INTF0: > + case DDP_COMPONENT_DP_INTF1: > + case DDP_COMPONENT_DPI0: > + case DDP_COMPONENT_DPI1: > + case DDP_COMPONENT_DSI0: > + case DDP_COMPONENT_DSI1: > + case DDP_COMPONENT_DSI2: > + case DDP_COMPONENT_DSI3: > + break; > + default: > + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", > + temp_path[idx - 1], ret); > + return -EINVAL; > + } > + > + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); > + if (!final_ddp_path) > + return -ENOMEM; > + > + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); > + > + /* Pipeline built! */ > + *out_path = final_ddp_path; > + *out_path_len = idx; > + > + return 0; > +} > + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-10-01 10:07 ` CK Hu (胡俊光) @ 2024-10-01 11:33 ` AngeloGioacchino Del Regno 2024-10-04 6:03 ` CK Hu (胡俊光) 0 siblings, 1 reply; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-10-01 11:33 UTC (permalink / raw) To: CK Hu (胡俊光), chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Il 01/10/24 12:07, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: >> It is impossible to add each and every possible DDP path combination >> for each and every possible combination of SoC and board: right now, >> this driver hardcodes configuration for 10 SoCs and this is going to >> grow larger and larger, and with new hacks like the introduction of >> mtk_drm_route which is anyway not enough for all final routes as the >> DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling >> DSC preventively doesn't work if the display doesn't support it, or >> others. >> >> Since practically all display IPs in MediaTek SoCs support being >> interconnected with different instances of other, or the same, IPs >> or with different IPs and in different combinations, the final DDP >> pipeline is effectively a board specific configuration. >> >> Implement OF graphs support to the mediatek-drm drivers, allowing to >> stop hardcoding the paths, and preventing this driver to get a huge >> amount of arrays for each board and SoC combination, also paving the >> way to share the same mtk_mmsys_driver_data between multiple SoCs, >> making it more straightforward to add support for new chips. >> >> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- > > [snip] > >> + >> +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) >> +{ >> + enum mtk_ovl_adaptor_comp_type type; >> + int ret; >> + >> + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); >> + if (ret) >> + return false; >> + >> + if (type >= OVL_ADAPTOR_TYPE_NUM) >> + return false; >> + >> + /* >> + * ETHDR and Padding are used exclusively in OVL Adaptor: if this >> + * component is not one of those, it's likely not an OVL Adaptor path. >> + */ > > I don't know your logic here. > The OVL Adaptor pipeline is: > > mdp_rdma -> padding ---+ +-------+ > Merge -> | | > mdp_rdma -> padding ---+ | | > | | > mdp_rdma -> padding ---+ | | > Merge -> | | > mdp_rdma -> padding ---+ | | > | ETHDR | > mdp_rdma -> padding ---+ | | > Merge -> | | > mdp_rdma -> padding ---+ | | > | | > mdp_rdma -> padding ---+ | | > Merge -> | | > mdp_rdma -> padding ---+ +-------+ > > So mdp_rdma and merge is not OVL Adaptor? > Yes, and in device tree, you express that exactly like you just pictured. OVL Adaptor is treated like a software component internally, and manages its own merge pipes exactly like before this commit. In case there will be any need to express OVL Adaptor as hardware component, it will be possible to do so with no modification to the bindings. > >> + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; >> +} >> + >> > > [snip] > >> + >> +/** >> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path >> + * @dev: The mediatek-drm device >> + * @cpath: CRTC Path relative to a VDO or MMSYS >> + * @out_path: Pointer to an array that will contain the new pipeline >> + * @out_path_len: Number of entries in the pipeline array >> + * >> + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending >> + * on the board-specific desired display configuration; this function walks >> + * through all of the output endpoints starting from a VDO or MMSYS hardware >> + * instance and builds the right pipeline as specified in device trees. >> + * >> + * Return: >> + * * %0 - Display HW Pipeline successfully built and validated >> + * * %-ENOENT - Display pipeline was not specified in device tree >> + * * %-EINVAL - Display pipeline built but validation failed >> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller >> + */ >> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, >> + const unsigned int **out_path, >> + unsigned int *out_path_len) >> +{ >> + struct device_node *next, *prev, *vdo = dev->parent->of_node; >> + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; >> + unsigned int *final_ddp_path; >> + unsigned short int idx = 0; >> + bool ovl_adaptor_comp_added = false; >> + int ret; >> + >> + /* Get the first entry for the temp_path array */ >> + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); >> + if (ret) { >> + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. This piece of code just avoids adding OVL_ADAPTOR more than once to the pipeline. > >> + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); >> + ovl_adaptor_comp_added = true; >> + } else { >> + if (next) >> + dev_err(dev, "Invalid component %pOF\n", next); >> + else >> + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); >> + >> + return ret; >> + } >> + } >> + idx++; >> + >> + /* >> + * Walk through port outputs until we reach the last valid mediatek-drm component. >> + * To be valid, this must end with an "invalid" component that is a display node. >> + */ >> + do { >> + prev = next; >> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); >> + of_node_put(prev); >> + if (ret) { >> + of_node_put(next); >> + break; >> + } >> + >> + /* >> + * If this is an OVL adaptor exclusive component and one of those >> + * was already added, don't add another instance of the generic >> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether >> + * to probe that component master driver of which only one instance >> + * is needed and possible. >> + */ >> + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > Finally, the path would be: > > mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... > Again, the path in the OF graph is expressed exactly like you said. Regards, Angelo > Regards, > CK > >> + if (!ovl_adaptor_comp_added) >> + ovl_adaptor_comp_added = true; >> + else >> + idx--; >> + } >> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); >> + >> + /* >> + * The device component might not be enabled: in that case, don't >> + * check the last entry and just report that the device is missing. >> + */ >> + if (ret == -ENODEV) >> + return ret; >> + >> + /* If the last entry is not a final display output, the configuration is wrong */ >> + switch (temp_path[idx - 1]) { >> + case DDP_COMPONENT_DP_INTF0: >> + case DDP_COMPONENT_DP_INTF1: >> + case DDP_COMPONENT_DPI0: >> + case DDP_COMPONENT_DPI1: >> + case DDP_COMPONENT_DSI0: >> + case DDP_COMPONENT_DSI1: >> + case DDP_COMPONENT_DSI2: >> + case DDP_COMPONENT_DSI3: >> + break; >> + default: >> + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", >> + temp_path[idx - 1], ret); >> + return -EINVAL; >> + } >> + >> + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); >> + if (!final_ddp_path) >> + return -ENOMEM; >> + >> + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); >> + >> + /* Pipeline built! */ >> + *out_path = final_ddp_path; >> + *out_path_len = idx; >> + >> + return 0; >> +} >> + ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-10-01 11:33 ` AngeloGioacchino Del Regno @ 2024-10-04 6:03 ` CK Hu (胡俊光) 2024-10-04 10:22 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 9+ messages in thread From: CK Hu (胡俊光) @ 2024-10-04 6:03 UTC (permalink / raw) To: AngeloGioacchino Del Regno, chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Hi, Angelo: On Tue, 2024-10-01 at 13:33 +0200, AngeloGioacchino Del Regno wrote: > Il 01/10/24 12:07, CK Hu (胡俊光) ha scritto: > > Hi, Angelo: > > > > On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: > > > It is impossible to add each and every possible DDP path combination > > > for each and every possible combination of SoC and board: right now, > > > this driver hardcodes configuration for 10 SoCs and this is going to > > > grow larger and larger, and with new hacks like the introduction of > > > mtk_drm_route which is anyway not enough for all final routes as the > > > DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling > > > DSC preventively doesn't work if the display doesn't support it, or > > > others. > > > > > > Since practically all display IPs in MediaTek SoCs support being > > > interconnected with different instances of other, or the same, IPs > > > or with different IPs and in different combinations, the final DDP > > > pipeline is effectively a board specific configuration. > > > > > > Implement OF graphs support to the mediatek-drm drivers, allowing to > > > stop hardcoding the paths, and preventing this driver to get a huge > > > amount of arrays for each board and SoC combination, also paving the > > > way to share the same mtk_mmsys_driver_data between multiple SoCs, > > > making it more straightforward to add support for new chips. > > > > > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > > > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > > > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > > > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > --- > > > > [snip] > > > > > + > > > +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) > > > +{ > > > + enum mtk_ovl_adaptor_comp_type type; > > > + int ret; > > > + > > > + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); > > > + if (ret) > > > + return false; > > > + > > > + if (type >= OVL_ADAPTOR_TYPE_NUM) > > > + return false; > > > + > > > + /* > > > + * ETHDR and Padding are used exclusively in OVL Adaptor: if this > > > + * component is not one of those, it's likely not an OVL Adaptor path. > > > + */ > > > > I don't know your logic here. > > The OVL Adaptor pipeline is: > > > > mdp_rdma -> padding ---+ +-------+ > > Merge -> | | > > mdp_rdma -> padding ---+ | | > > | | > > mdp_rdma -> padding ---+ | | > > Merge -> | | > > mdp_rdma -> padding ---+ | | > > | ETHDR | > > mdp_rdma -> padding ---+ | | > > Merge -> | | > > mdp_rdma -> padding ---+ | | > > | | > > mdp_rdma -> padding ---+ | | > > Merge -> | | > > mdp_rdma -> padding ---+ +-------+ > > > > So mdp_rdma and merge is not OVL Adaptor? > > > > Yes, and in device tree, you express that exactly like you just pictured. > > OVL Adaptor is treated like a software component internally, and manages > its own merge pipes exactly like before this commit. > > In case there will be any need to express OVL Adaptor as hardware component, > it will be possible to do so with no modification to the bindings. > > > > > > + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; > > > +} > > > + > > > > > > > [snip] > > > > > + > > > +/** > > > + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path > > > + * @dev: The mediatek-drm device > > > + * @cpath: CRTC Path relative to a VDO or MMSYS > > > + * @out_path: Pointer to an array that will contain the new pipeline > > > + * @out_path_len: Number of entries in the pipeline array > > > + * > > > + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending > > > + * on the board-specific desired display configuration; this function walks > > > + * through all of the output endpoints starting from a VDO or MMSYS hardware > > > + * instance and builds the right pipeline as specified in device trees. > > > + * > > > + * Return: > > > + * * %0 - Display HW Pipeline successfully built and validated > > > + * * %-ENOENT - Display pipeline was not specified in device tree > > > + * * %-EINVAL - Display pipeline built but validation failed > > > + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller > > > + */ > > > +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, > > > + const unsigned int **out_path, > > > + unsigned int *out_path_len) > > > +{ > > > + struct device_node *next, *prev, *vdo = dev->parent->of_node; > > > + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; > > > + unsigned int *final_ddp_path; > > > + unsigned short int idx = 0; > > > + bool ovl_adaptor_comp_added = false; > > > + int ret; > > > + > > > + /* Get the first entry for the temp_path array */ > > > + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); > > > + if (ret) { > > > + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > > > mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > > This piece of code just avoids adding OVL_ADAPTOR more than once to the pipeline. > > > > > > + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); > > > + ovl_adaptor_comp_added = true; > > > + } else { > > > + if (next) > > > + dev_err(dev, "Invalid component %pOF\n", next); > > > + else > > > + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); > > > + > > > + return ret; > > > + } > > > + } > > > + idx++; > > > + > > > + /* > > > + * Walk through port outputs until we reach the last valid mediatek-drm component. > > > + * To be valid, this must end with an "invalid" component that is a display node. > > > + */ > > > + do { > > > + prev = next; > > > + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); > > > + of_node_put(prev); > > > + if (ret) { > > > + of_node_put(next); > > > + break; > > > + } > > > + > > > + /* > > > + * If this is an OVL adaptor exclusive component and one of those > > > + * was already added, don't add another instance of the generic > > > + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether > > > + * to probe that component master driver of which only one instance > > > + * is needed and possible. > > > + */ > > > + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > > > merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > > Finally, the path would be: > > > > mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... > > > > Again, the path in the OF graph is expressed exactly like you said. I know the OF graph is expressed like I said. But I care about the path in driver should like this: static const unsigned int mt8195_mtk_ddp_ext[] = { DDP_COMPONENT_DRM_OVL_ADAPTOR, DDP_COMPONENT_MERGE5, DDP_COMPONENT_DP_INTF1, }; In OF graph, the first component is mdp_rdma and mtk_ovl_adaptor_is_comp_present() would return false for mdp_rdma. So I think this would make mtk_drm_of_ddp_path_build_one() return error and the path is not created. If I'm wrong, please explain how this code would result in the path like mt8195_mtk_ddp_ext[]. If you does not test this with mt8195 external display path, maybe we could just drop the code about OVL adaptor with a TODO comment. Regards, CK > > Regards, > Angelo > > > Regards, > > CK > > > > > + if (!ovl_adaptor_comp_added) > > > + ovl_adaptor_comp_added = true; > > > + else > > > + idx--; > > > + } > > > + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); > > > + > > > + /* > > > + * The device component might not be enabled: in that case, don't > > > + * check the last entry and just report that the device is missing. > > > + */ > > > + if (ret == -ENODEV) > > > + return ret; > > > + > > > + /* If the last entry is not a final display output, the configuration is wrong */ > > > + switch (temp_path[idx - 1]) { > > > + case DDP_COMPONENT_DP_INTF0: > > > + case DDP_COMPONENT_DP_INTF1: > > > + case DDP_COMPONENT_DPI0: > > > + case DDP_COMPONENT_DPI1: > > > + case DDP_COMPONENT_DSI0: > > > + case DDP_COMPONENT_DSI1: > > > + case DDP_COMPONENT_DSI2: > > > + case DDP_COMPONENT_DSI3: > > > + break; > > > + default: > > > + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", > > > + temp_path[idx - 1], ret); > > > + return -EINVAL; > > > + } > > > + > > > + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); > > > + if (!final_ddp_path) > > > + return -ENOMEM; > > > + > > > + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); > > > + > > > + /* Pipeline built! */ > > > + *out_path = final_ddp_path; > > > + *out_path_len = idx; > > > + > > > + return 0; > > > +} > > > + > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-10-04 6:03 ` CK Hu (胡俊光) @ 2024-10-04 10:22 ` AngeloGioacchino Del Regno 2024-10-07 6:57 ` CK Hu (胡俊光) 0 siblings, 1 reply; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-10-04 10:22 UTC (permalink / raw) To: CK Hu (胡俊光), chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Il 04/10/24 08:03, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Tue, 2024-10-01 at 13:33 +0200, AngeloGioacchino Del Regno wrote: >> Il 01/10/24 12:07, CK Hu (胡俊光) ha scritto: >>> Hi, Angelo: >>> >>> On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: >>>> It is impossible to add each and every possible DDP path combination >>>> for each and every possible combination of SoC and board: right now, >>>> this driver hardcodes configuration for 10 SoCs and this is going to >>>> grow larger and larger, and with new hacks like the introduction of >>>> mtk_drm_route which is anyway not enough for all final routes as the >>>> DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling >>>> DSC preventively doesn't work if the display doesn't support it, or >>>> others. >>>> >>>> Since practically all display IPs in MediaTek SoCs support being >>>> interconnected with different instances of other, or the same, IPs >>>> or with different IPs and in different combinations, the final DDP >>>> pipeline is effectively a board specific configuration. >>>> >>>> Implement OF graphs support to the mediatek-drm drivers, allowing to >>>> stop hardcoding the paths, and preventing this driver to get a huge >>>> amount of arrays for each board and SoC combination, also paving the >>>> way to share the same mtk_mmsys_driver_data between multiple SoCs, >>>> making it more straightforward to add support for new chips. >>>> >>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>> --- >>> >>> [snip] >>> >>>> + >>>> +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) >>>> +{ >>>> + enum mtk_ovl_adaptor_comp_type type; >>>> + int ret; >>>> + >>>> + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); >>>> + if (ret) >>>> + return false; >>>> + >>>> + if (type >= OVL_ADAPTOR_TYPE_NUM) >>>> + return false; >>>> + >>>> + /* >>>> + * ETHDR and Padding are used exclusively in OVL Adaptor: if this >>>> + * component is not one of those, it's likely not an OVL Adaptor path. >>>> + */ >>> >>> I don't know your logic here. >>> The OVL Adaptor pipeline is: >>> >>> mdp_rdma -> padding ---+ +-------+ >>> Merge -> | | >>> mdp_rdma -> padding ---+ | | >>> | | >>> mdp_rdma -> padding ---+ | | >>> Merge -> | | >>> mdp_rdma -> padding ---+ | | >>> | ETHDR | >>> mdp_rdma -> padding ---+ | | >>> Merge -> | | >>> mdp_rdma -> padding ---+ | | >>> | | >>> mdp_rdma -> padding ---+ | | >>> Merge -> | | >>> mdp_rdma -> padding ---+ +-------+ >>> >>> So mdp_rdma and merge is not OVL Adaptor? >>> >> >> Yes, and in device tree, you express that exactly like you just pictured. >> >> OVL Adaptor is treated like a software component internally, and manages >> its own merge pipes exactly like before this commit. >> >> In case there will be any need to express OVL Adaptor as hardware component, >> it will be possible to do so with no modification to the bindings. >> >>> >>>> + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; >>>> +} >>>> + >>>> >>> >>> [snip] >>> >>>> + >>>> +/** >>>> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path >>>> + * @dev: The mediatek-drm device >>>> + * @cpath: CRTC Path relative to a VDO or MMSYS >>>> + * @out_path: Pointer to an array that will contain the new pipeline >>>> + * @out_path_len: Number of entries in the pipeline array >>>> + * >>>> + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending >>>> + * on the board-specific desired display configuration; this function walks >>>> + * through all of the output endpoints starting from a VDO or MMSYS hardware >>>> + * instance and builds the right pipeline as specified in device trees. >>>> + * >>>> + * Return: >>>> + * * %0 - Display HW Pipeline successfully built and validated >>>> + * * %-ENOENT - Display pipeline was not specified in device tree >>>> + * * %-EINVAL - Display pipeline built but validation failed >>>> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller >>>> + */ >>>> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, >>>> + const unsigned int **out_path, >>>> + unsigned int *out_path_len) >>>> +{ >>>> + struct device_node *next, *prev, *vdo = dev->parent->of_node; >>>> + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; >>>> + unsigned int *final_ddp_path; >>>> + unsigned short int idx = 0; >>>> + bool ovl_adaptor_comp_added = false; >>>> + int ret; >>>> + >>>> + /* Get the first entry for the temp_path array */ >>>> + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); >>>> + if (ret) { >>>> + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { >>> >>> mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. >> >> This piece of code just avoids adding OVL_ADAPTOR more than once to the pipeline. >> >>> >>>> + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); >>>> + ovl_adaptor_comp_added = true; >>>> + } else { >>>> + if (next) >>>> + dev_err(dev, "Invalid component %pOF\n", next); >>>> + else >>>> + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); >>>> + >>>> + return ret; >>>> + } >>>> + } >>>> + idx++; >>>> + >>>> + /* >>>> + * Walk through port outputs until we reach the last valid mediatek-drm component. >>>> + * To be valid, this must end with an "invalid" component that is a display node. >>>> + */ >>>> + do { >>>> + prev = next; >>>> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); >>>> + of_node_put(prev); >>>> + if (ret) { >>>> + of_node_put(next); >>>> + break; >>>> + } >>>> + >>>> + /* >>>> + * If this is an OVL adaptor exclusive component and one of those >>>> + * was already added, don't add another instance of the generic >>>> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether >>>> + * to probe that component master driver of which only one instance >>>> + * is needed and possible. >>>> + */ >>>> + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { >>> >>> merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. >>> Finally, the path would be: >>> >>> mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... >>> >> >> Again, the path in the OF graph is expressed exactly like you said. > > I know the OF graph is expressed like I said. > But I care about the path in driver should like this: Ok, now I understand your concern. > > static const unsigned int mt8195_mtk_ddp_ext[] = { > DDP_COMPONENT_DRM_OVL_ADAPTOR, > DDP_COMPONENT_MERGE5, > DDP_COMPONENT_DP_INTF1, > }; > > In OF graph, the first component is mdp_rdma and mtk_ovl_adaptor_is_comp_present() would return false for mdp_rdma. > So I think this would make mtk_drm_of_ddp_path_build_one() return error and the path is not created. > If I'm wrong, please explain how this code would result in the path like mt8195_mtk_ddp_ext[]. > The MDP_RDMA usage in mtk_disp_ovl_adaptor is hardcoded: in function mtk_ovl_adaptor_layer_config(), the rdma_l/r are always OVL_ADAPTOR_MDP_RDMAx, then function mtk_ovl_adaptor_dma_dev_get(), always returns the MDP_RDMA0 component, same for mtk_ovl_adaptor_get_{num_formats,formats}() which always call mtk_mdp_rdma_get_formats() for OVL_ADAPTOR_MDP_RDMA0. I have just rechecked how I expressed the path for MT8195 Tomato, where the external display works with OF Graphs, and it was missing MDP_RDMA entirely. The path was ethdr -> merge -> dp_intf1 ... and it should be mdp_rdma -> (etc). Effectively, that is indeed wrong, as all of the steps must be expressed inside of the graph. Since the OVL Adaptor's RDMA instances' compatible strings do *not* collide with the others, as OVL Adaptor uses compatible mediatek,mt8195-vdo1-rdma, and the regular one uses compatible mediatek,mt8195-disp-rdma, we can resolve this issue by changing function mtk_ovl_adaptor_is_comp_present() from return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; to return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING || type == OVL_ADAPTOR_TYPE_MDP_RDMA; is that okay for you? > If you does not test this with mt8195 external display path, maybe we could just drop the code about OVL adaptor with a TODO comment. > And yes, as I said, external display paths were tested on 8195, actually both on Kontron i1200 by Michael Walle and on MT8195 Tomato by myself. Thanks again, Angelo > Regards, > CK > >> >> Regards, >> Angelo >> >>> Regards, >>> CK >>> >>>> + if (!ovl_adaptor_comp_added) >>>> + ovl_adaptor_comp_added = true; >>>> + else >>>> + idx--; >>>> + } >>>> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); >>>> + >>>> + /* >>>> + * The device component might not be enabled: in that case, don't >>>> + * check the last entry and just report that the device is missing. >>>> + */ >>>> + if (ret == -ENODEV) >>>> + return ret; >>>> + >>>> + /* If the last entry is not a final display output, the configuration is wrong */ >>>> + switch (temp_path[idx - 1]) { >>>> + case DDP_COMPONENT_DP_INTF0: >>>> + case DDP_COMPONENT_DP_INTF1: >>>> + case DDP_COMPONENT_DPI0: >>>> + case DDP_COMPONENT_DPI1: >>>> + case DDP_COMPONENT_DSI0: >>>> + case DDP_COMPONENT_DSI1: >>>> + case DDP_COMPONENT_DSI2: >>>> + case DDP_COMPONENT_DSI3: >>>> + break; >>>> + default: >>>> + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", >>>> + temp_path[idx - 1], ret); >>>> + return -EINVAL; >>>> + } >>>> + >>>> + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); >>>> + if (!final_ddp_path) >>>> + return -ENOMEM; >>>> + >>>> + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); >>>> + >>>> + /* Pipeline built! */ >>>> + *out_path = final_ddp_path; >>>> + *out_path_len = idx; >>>> + >>>> + return 0; >>>> +} >>>> + >> >> >> -- AngeloGioacchino Del Regno Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK Registered in England & Wales, no. 5513718 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-10-04 10:22 ` AngeloGioacchino Del Regno @ 2024-10-07 6:57 ` CK Hu (胡俊光) 2024-10-07 9:08 ` AngeloGioacchino Del Regno 0 siblings, 1 reply; 9+ messages in thread From: CK Hu (胡俊光) @ 2024-10-07 6:57 UTC (permalink / raw) To: AngeloGioacchino Del Regno, chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Hi, Angelo: On Fri, 2024-10-04 at 12:22 +0200, AngeloGioacchino Del Regno wrote: > Il 04/10/24 08:03, CK Hu (胡俊光) ha scritto: > > Hi, Angelo: > > > > On Tue, 2024-10-01 at 13:33 +0200, AngeloGioacchino Del Regno wrote: > > > Il 01/10/24 12:07, CK Hu (胡俊光) ha scritto: > > > > Hi, Angelo: > > > > > > > > On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: > > > > > It is impossible to add each and every possible DDP path combination > > > > > for each and every possible combination of SoC and board: right now, > > > > > this driver hardcodes configuration for 10 SoCs and this is going to > > > > > grow larger and larger, and with new hacks like the introduction of > > > > > mtk_drm_route which is anyway not enough for all final routes as the > > > > > DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling > > > > > DSC preventively doesn't work if the display doesn't support it, or > > > > > others. > > > > > > > > > > Since practically all display IPs in MediaTek SoCs support being > > > > > interconnected with different instances of other, or the same, IPs > > > > > or with different IPs and in different combinations, the final DDP > > > > > pipeline is effectively a board specific configuration. > > > > > > > > > > Implement OF graphs support to the mediatek-drm drivers, allowing to > > > > > stop hardcoding the paths, and preventing this driver to get a huge > > > > > amount of arrays for each board and SoC combination, also paving the > > > > > way to share the same mtk_mmsys_driver_data between multiple SoCs, > > > > > making it more straightforward to add support for new chips. > > > > > > > > > > Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> > > > > > Tested-by: Alexandre Mergnat <amergnat@baylibre.com> > > > > > Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> > > > > > Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > > > --- > > > > > > > > [snip] > > > > > > > > > + > > > > > +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) > > > > > +{ > > > > > + enum mtk_ovl_adaptor_comp_type type; > > > > > + int ret; > > > > > + > > > > > + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); > > > > > + if (ret) > > > > > + return false; > > > > > + > > > > > + if (type >= OVL_ADAPTOR_TYPE_NUM) > > > > > + return false; > > > > > + > > > > > + /* > > > > > + * ETHDR and Padding are used exclusively in OVL Adaptor: if this > > > > > + * component is not one of those, it's likely not an OVL Adaptor path. > > > > > + */ > > > > > > > > I don't know your logic here. > > > > The OVL Adaptor pipeline is: > > > > > > > > mdp_rdma -> padding ---+ +-------+ > > > > Merge -> | | > > > > mdp_rdma -> padding ---+ | | > > > > | | > > > > mdp_rdma -> padding ---+ | | > > > > Merge -> | | > > > > mdp_rdma -> padding ---+ | | > > > > | ETHDR | > > > > mdp_rdma -> padding ---+ | | > > > > Merge -> | | > > > > mdp_rdma -> padding ---+ | | > > > > | | > > > > mdp_rdma -> padding ---+ | | > > > > Merge -> | | > > > > mdp_rdma -> padding ---+ +-------+ > > > > > > > > So mdp_rdma and merge is not OVL Adaptor? > > > > > > > > > > Yes, and in device tree, you express that exactly like you just pictured. > > > > > > OVL Adaptor is treated like a software component internally, and manages > > > its own merge pipes exactly like before this commit. > > > > > > In case there will be any need to express OVL Adaptor as hardware component, > > > it will be possible to do so with no modification to the bindings. > > > > > > > > > > > > + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; > > > > > +} > > > > > + > > > > > > > > > > > > > [snip] > > > > > > > > > + > > > > > +/** > > > > > + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path > > > > > + * @dev: The mediatek-drm device > > > > > + * @cpath: CRTC Path relative to a VDO or MMSYS > > > > > + * @out_path: Pointer to an array that will contain the new pipeline > > > > > + * @out_path_len: Number of entries in the pipeline array > > > > > + * > > > > > + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending > > > > > + * on the board-specific desired display configuration; this function walks > > > > > + * through all of the output endpoints starting from a VDO or MMSYS hardware > > > > > + * instance and builds the right pipeline as specified in device trees. > > > > > + * > > > > > + * Return: > > > > > + * * %0 - Display HW Pipeline successfully built and validated > > > > > + * * %-ENOENT - Display pipeline was not specified in device tree > > > > > + * * %-EINVAL - Display pipeline built but validation failed > > > > > + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller > > > > > + */ > > > > > +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, > > > > > + const unsigned int **out_path, > > > > > + unsigned int *out_path_len) > > > > > +{ > > > > > + struct device_node *next, *prev, *vdo = dev->parent->of_node; > > > > > + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; > > > > > + unsigned int *final_ddp_path; > > > > > + unsigned short int idx = 0; > > > > > + bool ovl_adaptor_comp_added = false; > > > > > + int ret; > > > > > + > > > > > + /* Get the first entry for the temp_path array */ > > > > > + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); > > > > > + if (ret) { > > > > > + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > > > > > > > mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > > > > > > This piece of code just avoids adding OVL_ADAPTOR more than once to the pipeline. > > > > > > > > > > > > + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); > > > > > + ovl_adaptor_comp_added = true; > > > > > + } else { > > > > > + if (next) > > > > > + dev_err(dev, "Invalid component %pOF\n", next); > > > > > + else > > > > > + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); > > > > > + > > > > > + return ret; > > > > > + } > > > > > + } > > > > > + idx++; > > > > > + > > > > > + /* > > > > > + * Walk through port outputs until we reach the last valid mediatek-drm component. > > > > > + * To be valid, this must end with an "invalid" component that is a display node. > > > > > + */ > > > > > + do { > > > > > + prev = next; > > > > > + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); > > > > > + of_node_put(prev); > > > > > + if (ret) { > > > > > + of_node_put(next); > > > > > + break; > > > > > + } > > > > > + > > > > > + /* > > > > > + * If this is an OVL adaptor exclusive component and one of those > > > > > + * was already added, don't add another instance of the generic > > > > > + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether > > > > > + * to probe that component master driver of which only one instance > > > > > + * is needed and possible. > > > > > + */ > > > > > + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { > > > > > > > > merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. > > > > Finally, the path would be: > > > > > > > > mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... > > > > > > > > > > Again, the path in the OF graph is expressed exactly like you said. > > > > I know the OF graph is expressed like I said. > > But I care about the path in driver should like this: > > Ok, now I understand your concern. > > > > > static const unsigned int mt8195_mtk_ddp_ext[] = { > > DDP_COMPONENT_DRM_OVL_ADAPTOR, > > DDP_COMPONENT_MERGE5, > > DDP_COMPONENT_DP_INTF1, > > }; > > > > In OF graph, the first component is mdp_rdma and mtk_ovl_adaptor_is_comp_present() would return false for mdp_rdma. > > So I think this would make mtk_drm_of_ddp_path_build_one() return error and the path is not created. > > If I'm wrong, please explain how this code would result in the path like mt8195_mtk_ddp_ext[]. > > > > The MDP_RDMA usage in mtk_disp_ovl_adaptor is hardcoded: in function > mtk_ovl_adaptor_layer_config(), the rdma_l/r are always OVL_ADAPTOR_MDP_RDMAx, > then function mtk_ovl_adaptor_dma_dev_get(), always returns the MDP_RDMA0 > component, same for mtk_ovl_adaptor_get_{num_formats,formats}() which always > call mtk_mdp_rdma_get_formats() for OVL_ADAPTOR_MDP_RDMA0. > > I have just rechecked how I expressed the path for MT8195 Tomato, where the > external display works with OF Graphs, and it was missing MDP_RDMA entirely. > > The path was ethdr -> merge -> dp_intf1 ... and it should be mdp_rdma -> (etc). > > Effectively, that is indeed wrong, as all of the steps must be expressed > inside of the graph. > > Since the OVL Adaptor's RDMA instances' compatible strings do *not* collide > with the others, as OVL Adaptor uses compatible mediatek,mt8195-vdo1-rdma, > and the regular one uses compatible mediatek,mt8195-disp-rdma, we can resolve > this issue by changing function mtk_ovl_adaptor_is_comp_present() > > from > > return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; > > to > > return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING || > type == OVL_ADAPTOR_TYPE_MDP_RDMA; > > is that okay for you? I just want the path to be like mt8195_mtk_ddp_ext[]. If so, I'm ok. Regards, CK > > > If you does not test this with mt8195 external display path, maybe we could just drop the code about OVL adaptor with a TODO comment. > > > > And yes, as I said, external display paths were tested on 8195, actually both > on Kontron i1200 by Michael Walle and on MT8195 Tomato by myself. > > Thanks again, > Angelo > > > Regards, > > CK > > > > > > > > Regards, > > > Angelo > > > > > > > Regards, > > > > CK > > > > > > > > > + if (!ovl_adaptor_comp_added) > > > > > + ovl_adaptor_comp_added = true; > > > > > + else > > > > > + idx--; > > > > > + } > > > > > + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); > > > > > + > > > > > + /* > > > > > + * The device component might not be enabled: in that case, don't > > > > > + * check the last entry and just report that the device is missing. > > > > > + */ > > > > > + if (ret == -ENODEV) > > > > > + return ret; > > > > > + > > > > > + /* If the last entry is not a final display output, the configuration is wrong */ > > > > > + switch (temp_path[idx - 1]) { > > > > > + case DDP_COMPONENT_DP_INTF0: > > > > > + case DDP_COMPONENT_DP_INTF1: > > > > > + case DDP_COMPONENT_DPI0: > > > > > + case DDP_COMPONENT_DPI1: > > > > > + case DDP_COMPONENT_DSI0: > > > > > + case DDP_COMPONENT_DSI1: > > > > > + case DDP_COMPONENT_DSI2: > > > > > + case DDP_COMPONENT_DSI3: > > > > > + break; > > > > > + default: > > > > > + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", > > > > > + temp_path[idx - 1], ret); > > > > > + return -EINVAL; > > > > > + } > > > > > + > > > > > + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); > > > > > + if (!final_ddp_path) > > > > > + return -ENOMEM; > > > > > + > > > > > + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); > > > > > + > > > > > + /* Pipeline built! */ > > > > > + *out_path = final_ddp_path; > > > > > + *out_path_len = idx; > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths 2024-10-07 6:57 ` CK Hu (胡俊光) @ 2024-10-07 9:08 ` AngeloGioacchino Del Regno 0 siblings, 0 replies; 9+ messages in thread From: AngeloGioacchino Del Regno @ 2024-10-07 9:08 UTC (permalink / raw) To: CK Hu (胡俊光), chunkuang.hu@kernel.org Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org, sui.jingfeng@linux.dev, wenst@chromium.org, Sjoerd Simons, devicetree@vger.kernel.org, tzimmermann@suse.de, Shawn Sung (宋孝謙), mripard@kernel.org, Jitao Shi (石记涛), michael@walle.cc, daniel@ffwll.ch, p.zabel@pengutronix.de, conor+dt@kernel.org, maarten.lankhorst@linux.intel.com, robh@kernel.org, dri-devel@lists.freedesktop.org, airlied@gmail.com, krzysztof.kozlowski+dt@linaro.org, kernel@collabora.com, matthias.bgg@gmail.com, Yu-chang Lee (李禹璋), mwalle@kernel.org, linux-arm-kernel@lists.infradead.org, Alexandre Mergnat Il 07/10/24 08:57, CK Hu (胡俊光) ha scritto: > Hi, Angelo: > > On Fri, 2024-10-04 at 12:22 +0200, AngeloGioacchino Del Regno wrote: >> Il 04/10/24 08:03, CK Hu (胡俊光) ha scritto: >>> Hi, Angelo: >>> >>> On Tue, 2024-10-01 at 13:33 +0200, AngeloGioacchino Del Regno wrote: >>>> Il 01/10/24 12:07, CK Hu (胡俊光) ha scritto: >>>>> Hi, Angelo: >>>>> >>>>> On Tue, 2024-09-10 at 10:51 +0000, AngeloGioacchino Del Regno wrote: >>>>>> It is impossible to add each and every possible DDP path combination >>>>>> for each and every possible combination of SoC and board: right now, >>>>>> this driver hardcodes configuration for 10 SoCs and this is going to >>>>>> grow larger and larger, and with new hacks like the introduction of >>>>>> mtk_drm_route which is anyway not enough for all final routes as the >>>>>> DSI cannot be connected to MERGE if it's not a dual-DSI, or enabling >>>>>> DSC preventively doesn't work if the display doesn't support it, or >>>>>> others. >>>>>> >>>>>> Since practically all display IPs in MediaTek SoCs support being >>>>>> interconnected with different instances of other, or the same, IPs >>>>>> or with different IPs and in different combinations, the final DDP >>>>>> pipeline is effectively a board specific configuration. >>>>>> >>>>>> Implement OF graphs support to the mediatek-drm drivers, allowing to >>>>>> stop hardcoding the paths, and preventing this driver to get a huge >>>>>> amount of arrays for each board and SoC combination, also paving the >>>>>> way to share the same mtk_mmsys_driver_data between multiple SoCs, >>>>>> making it more straightforward to add support for new chips. >>>>>> >>>>>> Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>> Tested-by: Alexandre Mergnat <amergnat@baylibre.com> >>>>>> Acked-by: Sui Jingfeng <sui.jingfeng@linux.dev> >>>>>> Tested-by: Michael Walle <mwalle@kernel.org> # on kontron-sbc-i1200 >>>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >>>>>> --- >>>>> >>>>> [snip] >>>>> >>>>>> + >>>>>> +bool mtk_ovl_adaptor_is_comp_present(struct device_node *node) >>>>>> +{ >>>>>> + enum mtk_ovl_adaptor_comp_type type; >>>>>> + int ret; >>>>>> + >>>>>> + ret = ovl_adaptor_of_get_ddp_comp_type(node, &type); >>>>>> + if (ret) >>>>>> + return false; >>>>>> + >>>>>> + if (type >= OVL_ADAPTOR_TYPE_NUM) >>>>>> + return false; >>>>>> + >>>>>> + /* >>>>>> + * ETHDR and Padding are used exclusively in OVL Adaptor: if this >>>>>> + * component is not one of those, it's likely not an OVL Adaptor path. >>>>>> + */ >>>>> >>>>> I don't know your logic here. >>>>> The OVL Adaptor pipeline is: >>>>> >>>>> mdp_rdma -> padding ---+ +-------+ >>>>> Merge -> | | >>>>> mdp_rdma -> padding ---+ | | >>>>> | | >>>>> mdp_rdma -> padding ---+ | | >>>>> Merge -> | | >>>>> mdp_rdma -> padding ---+ | | >>>>> | ETHDR | >>>>> mdp_rdma -> padding ---+ | | >>>>> Merge -> | | >>>>> mdp_rdma -> padding ---+ | | >>>>> | | >>>>> mdp_rdma -> padding ---+ | | >>>>> Merge -> | | >>>>> mdp_rdma -> padding ---+ +-------+ >>>>> >>>>> So mdp_rdma and merge is not OVL Adaptor? >>>>> >>>> >>>> Yes, and in device tree, you express that exactly like you just pictured. >>>> >>>> OVL Adaptor is treated like a software component internally, and manages >>>> its own merge pipes exactly like before this commit. >>>> >>>> In case there will be any need to express OVL Adaptor as hardware component, >>>> it will be possible to do so with no modification to the bindings. >>>> >>>>> >>>>>> + return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; >>>>>> +} >>>>>> + >>>>>> >>>>> >>>>> [snip] >>>>> >>>>>> + >>>>>> +/** >>>>>> + * mtk_drm_of_ddp_path_build_one - Build a Display HW Pipeline for a CRTC Path >>>>>> + * @dev: The mediatek-drm device >>>>>> + * @cpath: CRTC Path relative to a VDO or MMSYS >>>>>> + * @out_path: Pointer to an array that will contain the new pipeline >>>>>> + * @out_path_len: Number of entries in the pipeline array >>>>>> + * >>>>>> + * MediaTek SoCs can use different DDP hardware pipelines (or paths) depending >>>>>> + * on the board-specific desired display configuration; this function walks >>>>>> + * through all of the output endpoints starting from a VDO or MMSYS hardware >>>>>> + * instance and builds the right pipeline as specified in device trees. >>>>>> + * >>>>>> + * Return: >>>>>> + * * %0 - Display HW Pipeline successfully built and validated >>>>>> + * * %-ENOENT - Display pipeline was not specified in device tree >>>>>> + * * %-EINVAL - Display pipeline built but validation failed >>>>>> + * * %-ENOMEM - Failure to allocate pipeline array to pass to the caller >>>>>> + */ >>>>>> +static int mtk_drm_of_ddp_path_build_one(struct device *dev, enum mtk_crtc_path cpath, >>>>>> + const unsigned int **out_path, >>>>>> + unsigned int *out_path_len) >>>>>> +{ >>>>>> + struct device_node *next, *prev, *vdo = dev->parent->of_node; >>>>>> + unsigned int temp_path[DDP_COMPONENT_DRM_ID_MAX] = { 0 }; >>>>>> + unsigned int *final_ddp_path; >>>>>> + unsigned short int idx = 0; >>>>>> + bool ovl_adaptor_comp_added = false; >>>>>> + int ret; >>>>>> + >>>>>> + /* Get the first entry for the temp_path array */ >>>>>> + ret = mtk_drm_of_get_ddp_ep_cid(vdo, 0, cpath, &next, &temp_path[idx]); >>>>>> + if (ret) { >>>>>> + if (next && temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { >>>>> >>>>> mdp_rdma would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. >>>> >>>> This piece of code just avoids adding OVL_ADAPTOR more than once to the pipeline. >>>> >>>>> >>>>>> + dev_dbg(dev, "Adding OVL Adaptor for %pOF\n", next); >>>>>> + ovl_adaptor_comp_added = true; >>>>>> + } else { >>>>>> + if (next) >>>>>> + dev_err(dev, "Invalid component %pOF\n", next); >>>>>> + else >>>>>> + dev_err(dev, "Cannot find first endpoint for path %d\n", cpath); >>>>>> + >>>>>> + return ret; >>>>>> + } >>>>>> + } >>>>>> + idx++; >>>>>> + >>>>>> + /* >>>>>> + * Walk through port outputs until we reach the last valid mediatek-drm component. >>>>>> + * To be valid, this must end with an "invalid" component that is a display node. >>>>>> + */ >>>>>> + do { >>>>>> + prev = next; >>>>>> + ret = mtk_drm_of_get_ddp_ep_cid(next, 1, cpath, &next, &temp_path[idx]); >>>>>> + of_node_put(prev); >>>>>> + if (ret) { >>>>>> + of_node_put(next); >>>>>> + break; >>>>>> + } >>>>>> + >>>>>> + /* >>>>>> + * If this is an OVL adaptor exclusive component and one of those >>>>>> + * was already added, don't add another instance of the generic >>>>>> + * DDP_COMPONENT_OVL_ADAPTOR, as this is used only to decide whether >>>>>> + * to probe that component master driver of which only one instance >>>>>> + * is needed and possible. >>>>>> + */ >>>>>> + if (temp_path[idx] == DDP_COMPONENT_DRM_OVL_ADAPTOR) { >>>>> >>>>> merge would not be DDP_COMPONENT_DRM_OVL_ADAPTOR. >>>>> Finally, the path would be: >>>>> >>>>> mdp_rdma -> ovl adaptor (padding) -> merge -> (ethdr is skipped here) ... >>>>> >>>> >>>> Again, the path in the OF graph is expressed exactly like you said. >>> >>> I know the OF graph is expressed like I said. >>> But I care about the path in driver should like this: >> >> Ok, now I understand your concern. >> >>> >>> static const unsigned int mt8195_mtk_ddp_ext[] = { >>> DDP_COMPONENT_DRM_OVL_ADAPTOR, >>> DDP_COMPONENT_MERGE5, >>> DDP_COMPONENT_DP_INTF1, >>> }; >>> >>> In OF graph, the first component is mdp_rdma and mtk_ovl_adaptor_is_comp_present() would return false for mdp_rdma. >>> So I think this would make mtk_drm_of_ddp_path_build_one() return error and the path is not created. >>> If I'm wrong, please explain how this code would result in the path like mt8195_mtk_ddp_ext[]. >>> >> >> The MDP_RDMA usage in mtk_disp_ovl_adaptor is hardcoded: in function >> mtk_ovl_adaptor_layer_config(), the rdma_l/r are always OVL_ADAPTOR_MDP_RDMAx, >> then function mtk_ovl_adaptor_dma_dev_get(), always returns the MDP_RDMA0 >> component, same for mtk_ovl_adaptor_get_{num_formats,formats}() which always >> call mtk_mdp_rdma_get_formats() for OVL_ADAPTOR_MDP_RDMA0. >> >> I have just rechecked how I expressed the path for MT8195 Tomato, where the >> external display works with OF Graphs, and it was missing MDP_RDMA entirely. >> >> The path was ethdr -> merge -> dp_intf1 ... and it should be mdp_rdma -> (etc). >> >> Effectively, that is indeed wrong, as all of the steps must be expressed >> inside of the graph. >> >> Since the OVL Adaptor's RDMA instances' compatible strings do *not* collide >> with the others, as OVL Adaptor uses compatible mediatek,mt8195-vdo1-rdma, >> and the regular one uses compatible mediatek,mt8195-disp-rdma, we can resolve >> this issue by changing function mtk_ovl_adaptor_is_comp_present() >> >> from >> >> return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING; >> >> to >> >> return type == OVL_ADAPTOR_TYPE_ETHDR || type == OVL_ADAPTOR_TYPE_PADDING || >> type == OVL_ADAPTOR_TYPE_MDP_RDMA; >> >> is that okay for you? > > I just want the path to be like mt8195_mtk_ddp_ext[]. If so, I'm ok. > Yes, that makes the path that you described to be exactly like mt8195_mtk_ddp_ext[]. I will send a v11 later today. Cheers, Angelo > Regards, > CK > >> >>> If you does not test this with mt8195 external display path, maybe we could just drop the code about OVL adaptor with a TODO comment. >>> >> >> And yes, as I said, external display paths were tested on 8195, actually both >> on Kontron i1200 by Michael Walle and on MT8195 Tomato by myself. >> >> Thanks again, >> Angelo >> >>> Regards, >>> CK >>> >>>> >>>> Regards, >>>> Angelo >>>> >>>>> Regards, >>>>> CK >>>>> >>>>>> + if (!ovl_adaptor_comp_added) >>>>>> + ovl_adaptor_comp_added = true; >>>>>> + else >>>>>> + idx--; >>>>>> + } >>>>>> + } while (++idx < DDP_COMPONENT_DRM_ID_MAX); >>>>>> + >>>>>> + /* >>>>>> + * The device component might not be enabled: in that case, don't >>>>>> + * check the last entry and just report that the device is missing. >>>>>> + */ >>>>>> + if (ret == -ENODEV) >>>>>> + return ret; >>>>>> + >>>>>> + /* If the last entry is not a final display output, the configuration is wrong */ >>>>>> + switch (temp_path[idx - 1]) { >>>>>> + case DDP_COMPONENT_DP_INTF0: >>>>>> + case DDP_COMPONENT_DP_INTF1: >>>>>> + case DDP_COMPONENT_DPI0: >>>>>> + case DDP_COMPONENT_DPI1: >>>>>> + case DDP_COMPONENT_DSI0: >>>>>> + case DDP_COMPONENT_DSI1: >>>>>> + case DDP_COMPONENT_DSI2: >>>>>> + case DDP_COMPONENT_DSI3: >>>>>> + break; >>>>>> + default: >>>>>> + dev_err(dev, "Invalid display hw pipeline. Last component: %d (ret=%d)\n", >>>>>> + temp_path[idx - 1], ret); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + >>>>>> + final_ddp_path = devm_kmemdup(dev, temp_path, idx * sizeof(temp_path[0]), GFP_KERNEL); >>>>>> + if (!final_ddp_path) >>>>>> + return -ENOMEM; >>>>>> + >>>>>> + dev_dbg(dev, "Display HW Pipeline built with %d components.\n", idx); >>>>>> + >>>>>> + /* Pipeline built! */ >>>>>> + *out_path = final_ddp_path; >>>>>> + *out_path_len = idx; >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>> >>>> >>>> >> >> -- AngeloGioacchino Del Regno Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK Registered in England & Wales, no. 5513718 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-10-07 9:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20240910105054.125005-1-angelogioacchino.delregno@collabora.com>
2024-09-10 10:51 ` [PATCH v10 1/3] dt-bindings: display: mediatek: Add OF graph support for board path AngeloGioacchino Del Regno
2024-09-10 10:51 ` [PATCH v10 2/3] dt-bindings: arm: mediatek: mmsys: " AngeloGioacchino Del Regno
2024-09-10 10:51 ` [PATCH v10 3/3] drm/mediatek: Implement OF graphs support for display paths AngeloGioacchino Del Regno
2024-10-01 10:07 ` CK Hu (胡俊光)
2024-10-01 11:33 ` AngeloGioacchino Del Regno
2024-10-04 6:03 ` CK Hu (胡俊光)
2024-10-04 10:22 ` AngeloGioacchino Del Regno
2024-10-07 6:57 ` CK Hu (胡俊光)
2024-10-07 9:08 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).