| Message ID | 1442592722-29004-2-git-send-email-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 18, 2015 at 11:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > From: CK Hu <ck.hu@mediatek.com> > > Add device tree binding documentation for the display subsystem in > Mediatek MT8173 SoCs. > > Signed-off-by: CK Hu <ck.hu@mediatek.com> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > --- > .../bindings/drm/mediatek/mediatek,disp.txt | 131 +++++++++++++++++++++ > .../bindings/drm/mediatek/mediatek,dsi.txt | 29 +++++ > 2 files changed, 160 insertions(+) > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > new file mode 100644 > index 0000000..a3811bd > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > @@ -0,0 +1,131 @@ > +Mediatek display subsystem > +========================== > + > +The Mediatek display subsystem consists of various DISP function blocks in the > +MMSYS register space. The connections between them can be configured by output > +and input selectors in the MMSYS_CONFIG register space and register updates can > +be synchronized to video frame boundaries with help of a DISP_MUTEX function > +block. > + > +The display-subsystem node binds together all individual device nodes that > +comprise the DISP subsystem. > + > +Required properties: > + > +- compatible: "mediatek,<chip>-disp" > +- components: Should contain a list of phandles pointing to the DISP function > + block device nodes. > +- component-names: Should contain the name of the function block pointed to > + by the components phandle of the same index. NAK. Group these nodes under a parent node, use of-graph or just don't put this into DT. Don't invent a new way. > +- mmsys-config: Should contain a phandle pointing to the MMSYS node. > +- disp-mutex: Should contain a phandle pointing to the DISP_MUTEX node. > + > +Example: > + > +display-subsystem { > + compatible = "mediatek,mt8173-disp"; > + components = <&ovl0>, <&rdma0>, <&color0>, <&aal>, > + <&ufoe>, <&dsi0>, <&od>; > + component-names = "ovl0", "rdma0", "color0", "aal", > + "ufoe", "dsi0", "od"; > + mmsys-config = <&mmsys>; > + disp-mutex = <&mutex>; > +}; > + > +DISP function blocks > +==================== > + > +A display stream starts at a source function block that reads pixel data from > +memory and ends with a sink function block that drives pixels on a display > +interface, or writes pixels back to memory. All DISP function blocks have > +their own register space, interrupt, and clock gate. The blocks that can > +access memory additionally have to list the IOMMU and local arbiter they are > +connected to. > + > +For a description of the display interface sink function blocks, see > +Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > + > +Required properties (all function blocks): > +- compatible: "mediatek,<chip>-disp-<function>", one of > + "mediatek,<chip>-disp-ovl" - overlay (4 layers, blending, csc) > + "mediatek,<chip>-disp-rdma" - read DMA / line buffer > + "mediatek,<chip>-disp-color" - color processor > + "mediatek,<chip>-disp-aal" - adaptive ambient light controller > + "mediatek,<chip>-disp-gamma" - gamma correction > + "mediatek,<chip>-disp-ufoe" - data compression engine > + "mediatek,<chip>-disp-mutex" - display mutex > + "mediatek,<chip>-disp-od" - overdrive > +- reg: Physical base address and length of the function block register space > +- interrupts: The interrupt signal from the function block. > +- clocks: device clocks > + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. > +- compatible: "mediatek,<chip>-ddp" > +- power-domains: a phandle to DDP power domain node. > + > +Required properties (DMA function blocks): > +- compatible: Should be one of > + "mediatek,<chip>-disp-ovl" > + "mediatek,<chip>-disp-rdma" > +- larb: Should a phandles pointing to the local arbiter device as defined in > + Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt > +- iommus: required a iommu node > + > +Examples: > + > +ovl0: ovl@1400c000 { > + compatible = "mediatek,mt8173-disp-ovl"; > + reg = <0 0x1400c000 0 0x1000>; > + interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_OVL0>; > + iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>; > +}; > + > +rdma0: rdma@1400e000 { > + compatible = "mediatek,mt8173-disp-rdma"; > + reg = <0 0x1400e000 0 0x1000>; > + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_RDMA0>; > + iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_RDMA0>; > +}; > + > +color0: color@14013000 { > + compatible = "mediatek,mt8173-disp-color"; > + reg = <0 0x14013000 0 0x1000>; > + interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_COLOR0>; > +}; > + > +aal: aal@14015000 { > + compatible = "mediatek,mt8173-disp-aal"; > + reg = <0 0x14015000 0 0x1000>; > + interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_AAL>; > +}; > + > +gamma: gamma@14016000 { > + compatible = "mediatek,mt8173-disp-gamma"; > + reg = <0 0x14016000 0 0x1000>; > + interrupts = <GIC_SPI 190 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_GAMMA>; > +}; > + > +ufoe: ufoe@1401a000 { > + compatible = "mediatek,mt8173-disp-ufoe"; > + reg = <0 0x1401a000 0 0x1000>; > + interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_LOW>; > + clocks = <&mmsys CLK_MM_DISP_UFOE>; > +}; > + > +mutex: mutex@14020000 { > + compatible = "mediatek,mt8173-disp-mutex"; > + reg = <0 0x14020000 0 0x1000>; > + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_LOW>; > + power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; > + clocks = <&mmsys CLK_MM_MUTEX_32K>; > +}; > + > +od: od@14023000 { > + compatible = "mediatek,mt8173-disp-od"; > + reg = <0 0x14023000 0 0x1000>; > + clocks = <&mmsys CLK_MM_DISP_OD>; > +}; > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > new file mode 100644 > index 0000000..e892ef1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > @@ -0,0 +1,29 @@ > +Mediatek DSI Device > +=================== > + > +The Mediatek DSI function block is a sink of the display subsystem and can > +drive up to 4-lane MIPI DSI output. Two DSIs can be synchronized for dual- > +channel output. > + > +Required properties: > +- compatible: "mediatek,<chip>-dsi" > +- reg: Physical base address and length of the controller's registers > +- clocks: device clocks > + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. > +- clock-names: must contain "engine" and "digital". This leaves wondering which one is used for DSI bit clock. > + > +Example: > + > +dsi0: dsi@1401b000 { > + compatible = "mediatek,mt8173-dsi"; > + reg = <0 0x1401b000 0 0x1000>, /* DSI0 */ > + <0 0x10215000 0 0x1000>; /* MIPI_TX0 */ > + clocks = <&mmsys MM_DSI0_ENGINE>, <&mmsys MM_DSI0_DIGITAL>; > + clock-names = "engine", "digital"; > + > + port { Missing from the binding description. > + dsi0_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > +}; > -- > 2.5.1 >
Hi Rob, thank you for the comments. Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring: [...] > > +The display-subsystem node binds together all individual device nodes that > > +comprise the DISP subsystem. > > + > > +Required properties: > > + > > +- compatible: "mediatek,<chip>-disp" > > > +- components: Should contain a list of phandles pointing to the DISP function > > + block device nodes. > > +- component-names: Should contain the name of the function block pointed to > > + by the components phandle of the same index. > > NAK. Group these nodes under a parent node, use of-graph or just don't > put this into DT. Don't invent a new way. Ok. The reason I haven't grouped all the display nodes together in the first place is that they aren't contiguous in the register space. So instead of: ovl@1400c000 { compatible = "mediatek,mt8173-disp-ovl"; }; ... ufoe@1401a000 { compatible = "mediatek,mt8173-disp-ufoe"; }; pwm@1401e000 { compatible = "mediatek,mt8173-disp-pwm"; }; larb@14021000 { compatible = "mediatek,mt8173-smi-larb"; }; od@14023000 { compatible = "mediatek,mt8173-disp-od"; }; We'd have: display-subsystem@1400c00 { compatible = "mediatek,mt8173-disp", "simple-bus"; ovl@1400c000 { compatible = "mediatek,mt8173-disp-ovl"; }; ... ufoe@1401a000 { compatible = "mediatek,mt8173-disp-ufoe"; }; od@14023000 { compatible = "mediatek,mt8173-disp-od"; }; }; pwm@1401e000 { compatible = "mediatek,mt8173-disp-pwm"; }; larb@14021000 { compatible = "mediatek,mt8173-smi-larb"; }; Note how the display-subsystem node overlaps the larb node. Is that acceptable? [...] > > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > > new file mode 100644 > > index 0000000..e892ef1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > > @@ -0,0 +1,29 @@ > > +Mediatek DSI Device > > +=================== > > + > > +The Mediatek DSI function block is a sink of the display subsystem and can > > +drive up to 4-lane MIPI DSI output. Two DSIs can be synchronized for dual- > > +channel output. > > + > > +Required properties: > > +- compatible: "mediatek,<chip>-dsi" > > +- reg: Physical base address and length of the controller's registers > > +- clocks: device clocks > > + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. > > +- clock-names: must contain "engine" and "digital". > > This leaves wondering which one is used for DSI bit clock. The MIPI_TX0 module controls the MIPI D-PHY. It contains a PLL that provides the bit clock to the DSI module. From the documentation, it looks to me like the AP_PLL_CON0[6] bit in the mediatek,mt8173-apmixedsys region gates the 26 MHz reference clock to MIPI_TX. It is enabled by default. Currently there is no gate clock registered for that bit. Can somebody confirm that this gate clock should be added to the mediatek,mt8173-apmixedsys bindings? > > + > > +Example: > > + > > +dsi0: dsi@1401b000 { > > + compatible = "mediatek,mt8173-dsi"; > > + reg = <0 0x1401b000 0 0x1000>, /* DSI0 */ > > + <0 0x10215000 0 0x1000>; /* MIPI_TX0 */ Thinking about it, MIPI_TX0 is for PHY control. Should this be moved into its own node and the phy bindings used to let the DSI driver find it? > > + clocks = <&mmsys MM_DSI0_ENGINE>, <&mmsys MM_DSI0_DIGITAL>; > > + clock-names = "engine", "digital"; > > + > > + port { > > Missing from the binding description. Thanks, I'll fix that next round. > > + dsi0_out: endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > +}; best regards Philipp
Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring: > On Fri, Sep 18, 2015 at 11:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > From: CK Hu <ck.hu@mediatek.com> > > > > Add device tree binding documentation for the display subsystem in > > Mediatek MT8173 SoCs. > > > > Signed-off-by: CK Hu <ck.hu@mediatek.com> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > --- > > .../bindings/drm/mediatek/mediatek,disp.txt | 131 +++++++++++++++++++++ > > .../bindings/drm/mediatek/mediatek,dsi.txt | 29 +++++ > > 2 files changed, 160 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > > > > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > > new file mode 100644 > > index 0000000..a3811bd > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > > @@ -0,0 +1,131 @@ > > +Mediatek display subsystem > > +========================== > > + > > +The Mediatek display subsystem consists of various DISP function blocks in the > > +MMSYS register space. The connections between them can be configured by output > > +and input selectors in the MMSYS_CONFIG register space and register updates can > > +be synchronized to video frame boundaries with help of a DISP_MUTEX function > > +block. > > + > > +The display-subsystem node binds together all individual device nodes that > > +comprise the DISP subsystem. > > + > > +Required properties: > > + > > +- compatible: "mediatek,<chip>-disp" > > > +- components: Should contain a list of phandles pointing to the DISP function > > + block device nodes. > > +- component-names: Should contain the name of the function block pointed to > > + by the components phandle of the same index. > > NAK. Group these nodes under a parent node, use of-graph or just don't > put this into DT. Don't invent a new way. If I connect the DISP nodes using of-graph bindings, the full graph will look somewhat like this (including the currently unused function blocks, all properties but ports and endpoints removed for brevity): ovl0@1400c000 { port { ovl0_to_color0: endpoint@0 { remote-endpoint = <&color0_from_ovl0>; }; ovl0_to_wdma0: endpoint@1 { remote-endpoint = <&wdma0_from_ovl0>; }; }; }; ovl1@1400d000 { port { ovl1_to_color1: endpoint@0 { remote-endpoint = <&color1_from_ovl1>; }; ovl1_to_wdma1: endpoint@1 { remote-endpoint = <&wdma1_from_ovl1>; }; }; }; rdma0@1400e000 { port@0 { rdma0_from_od: endpoint { remote-endpoint = <&od_to_rdma0>; }; }; port@1 { rdma0_to_ufoe: endpoint@0 { remote-endpoint = <&ufoe_from_rdma0>; }; rdma0_to_split0: endpoint@1 { remote-endpoint = <&split0_from_rdma0>; }; rdma0_to_color0: endpoint@2 { remote-endpoint = <&color0_from_rdma0>; }; }; }; rdma1@1400f000 { port@0 { rdma1_from_gamma: endpoint { remote-endpoint = <&gamma_to_rdma1>; }; }; port@1 { rdma1_to_dsi0: endpoint@0 { remote-endpoint = <&dsi0_from_rdma1>; }; rdma1_to_dsi1: endpoint@1 { remote-endpoint = <&dsi1_from_rdma1>; }; rdma1_to_dpi0: endpoint@2 { remote-endpoint = <&dpi0_from_rdma1>; }; rdma1_to_color1: endpoint@3 { remote-endpoint = <&color1_from_rdma1>; }; }; }; rdma2@14010000 { port { rdma2_to_dsi1: endpoint@0 { remote-endpoint = <&dsi1_from_rdma2>; }; rdma2_to_dpi0: endpoint@1 { remote-endpoint = <&dpi0_from_rdma2>; }; }; }; wdma0@14011000 { port { wdma0_from_ovl0: endpoint@0 { remote-endpoint = <&ovl0_to_wdma0>; }; wdma0_from_od: endpoint@1 { remote-endpoint = <&od_to_wdma0>; }; wdma0_from_ufoe: endpoint@2 { remote-endpoint = <&ufoe_to_wdma0>; }; }; }; wdma1@14012000 { port { wdma1_from_ovl1: endpoint@0 { remote-endpoint = <&ovl1_to_wdma1>; }; wdma1_from_gamma: endpoint@1 { remote-endpoint = <&gamma_to_wdma1>; }; }; }; color0@14013000 { port@0 { color0_from_rdma0: endpoint@0 { remote-endpoint = <&rdma0_to_color0>; }; color0_from_ovl0: endpoint@1 { remote-endpoint = <&ovl0_to_color0>; }; }; port@1 { color0_to_aal: endpoint@0 { remote-endpoint = <&aal_from_color0>; }; color0_to_merge: endpoint@1 { remote-endpoint = <&merge_from_color0>; }; }; }; color1@14014000 { port@0 { color1_from_rdma1: endpoint@0 { remote-endpoint = <&rdma1_to_color1>; }; color1_from_ovl1: endpoint@1 { remote-endpoint = <&ovl1_to_color1>; }; }; port@1 { color1_to_gamma: endpoint@0 { remote-endpoint = <&gamma_from_color1>; }; color1_to_merge: endpoint@1 { remote-endpoint = <&merge_from_color1>; }; }; }; aal@14015000 { port@0 { aal_from_color0: endpoint@0 { remote-endpoint = <&color0_to_aal>; }; aal_from_merge: endpoint@1 { remote-endpoint = <&merge_to_aal>; }; }; port@1 { aal_to_od: endpoint { remote-endpoint = <&od_from_aal>; }; }; }; gamma@14016000 { port@0 { gamma_from_color1: endpoint { remote-endpoint = <&color1_to_gamma>; }; }; port@1 { gamma_to_rdma1: endpoint@0 { remote-endpoint = <&rdma1_from_gamma>; }; gamma_to_dsi0: endpoint@1 { remote-endpoint = <&dsi0_from_gamma>; }; gamma_to_dsi1: endpoint@2 { remote-endpoint = <&dsi1_from_gamma>; }; gamma_to_dpi0: endpoint@3 { remote-endpoint = <&dpi0_from_gamma>; }; gamma_to_wdma1: endpoint@4 { remote-endpoint = <&wdma1_from_gamma>; }; }; }; merge@14017000 { port@0 { merge_from_color0: endpoint { remote-endpoint = <&color0_to_merge>; }; }; port@1 { merge_from_color1: endpoint { remote-endpoint = <&color1_to_merge>; }; }; port@2 { merge_to_aal: endpoint { remote-endpoint = <&aal_from_merge>; }; }; }; split0@14018000 { port@0 { split0_from_rdma0: endpoint@0 { remote-endpoint = <&rdma0_to_split0>; }; split0_from_od: endpoint@1 { remote-endpoint = <&od_to_split0>; }; }; port@1 { split0_to_ufoe: endpoint { remote-endpoint = <&ufoe_from_split0>; }; }; port@2 { split0_to_dsi0: endpoint@0 { remote-endpoint = <&dsi0_from_split0>; }; split0_to_dsi1: endpoint@1 { remote-endpoint = <&dsi1_from_split0>; }; split0_to_dpi0: endpoint@2 { remote-endpoint = <&dpi0_from_split0>; }; }; }; split1@14019000 { port@0 { split1_from_ufoe: endpoint { remote-endpoint = <&ufoe_to_split1>; }; }; port@1 { split1_to_dsi0: endpoint { remote-endpoint = <&dsi0_from_split1>; }; }; port@2 { split1_to_dsi1: endpoint { remote-endpoint = <&dsi1_from_split1>; }; }; }; ufoe@1401a000 { port@0 { ufoe_from_rdma0: endpoint@0 { remote-endpoint = <&rdma0_to_ufoe>; }; ufoe_from_od: endpoint@1 { remote-endpoint = <&od_to_ufoe>; }; ufoe_from_split0: endpoint@2 { remote-endpoint = <&split0_to_ufoe>; }; }; port@1 { ufoe_to_dsi0: endpoint@0 { remote-endpoint = <&dsi0_from_ufoe>; }; ufoe_to_split1: endpoint@1 { remote-endpoint = <&split1_from_ufoe>; }; ufoe_to_dpi0: endpoint@2 { remote-endpoint = <&dpi0_from_ufoe>; }; ufoe_to_wdma0: endpoint@3 { remote-endpoint = <&wdma0_from_ufoe>; }; ufoe_to_dsi1: endpoint@4 { remote-endpoint = <&dsi1_from_ufoe>; }; }; }; dsi0: dsi0@1401b000 { port@0 { dsi0_from_ufoe: endpoint@0 { remote-endpoint = <&ufoe_to_dsi0>; }; dsi0_from_split1: endpoint@1 { remote-endpoint = <&split1_to_dsi0>; }; dsi0_from_rdma1: endpoint@2 { remote-endpoint = <&rdma1_to_dsi0>; }; dsi0_from_gamma: endpoint@3 { remote-endpoint = <&gamma_to_dsi0>; }; dsi0_from_split0: endpoint@4 { remote-endpoint = <&split0_to_dsi0>; }; }; }; dsi1: dsi1@1401c000 { port@0 { dsi1_from_split1: endpoint@0 { remote-endpoint = <&split1_to_dsi1>; }; dsi1_from_rdma1: endpoint@1 { remote-endpoint = <&rdma1_to_dsi1>; }; dsi1_from_gamma: endpoint@2 { remote-endpoint = <&gamma_to_dsi1>; }; dsi1_from_split0: endpoint@3 { remote-endpoint = <&split0_to_dsi1>; }; dsi1_from_rdma2: endpoint@4 { remote-endpoint = <&rdma2_to_dsi1>; }; dsi1_from_ufoe: endpoint@5 { remote-endpoint = <&ufoe_to_dsi1>; }; }; }; dpi0: dpi0@1401d000 { port@0 { dpi0_from_ufoe: endpoint@0 { remote-endpoint = <&ufoe_to_dpi0>; }; dpi0_from_rdma1: endpoint@1 { remote-endpoint = <&rdma1_to_dpi0>; }; dpi0_from_gamma: endpoint@2 { remote-endpoint = <&gamma_to_dpi0>; }; dpi0_from_split0: endpoint@3 { remote-endpoint = <&split0_to_dpi0>; }; dpi0_from_rdma2: endpoint@4 { remote-endpoint = <&rdma2_to_dpi0>; }; }; }; od@14023000 { port@0 { od_from_aal: endpoint { remote-endpoint = <&aal_to_od>; }; }; port@1 { od_to_rdma0: endpoint@0 { remote-endpoint = <&rdma0_from_od>; }; od_to_ufoe: endpoint@1 { remote-endpoint = <&ufoe_from_od>; }; od_to_split0: endpoint@2 { remote-endpoint = <&split0_from_od>; }; od_to_wdma0: endpoint@3 { remote-endpoint = <&wdma0_from_od>; }; }; }; Adding the graph to the mediatek oak-rev3 device tree increases the size from currently 33 KiB to 41.5 KiB. As a proponent of the of-graph bindings I certainly like to describe the hardware connections using DT, but I'm not sure if maybe this is a bit verbose. regards Philipp
On Wed, Sep 30, 2015 at 10:30 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring: >> On Fri, Sep 18, 2015 at 11:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> > From: CK Hu <ck.hu@mediatek.com> >> > >> > Add device tree binding documentation for the display subsystem in >> > Mediatek MT8173 SoCs. >> > >> > Signed-off-by: CK Hu <ck.hu@mediatek.com> >> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> > --- >> > .../bindings/drm/mediatek/mediatek,disp.txt | 131 +++++++++++++++++++++ >> > .../bindings/drm/mediatek/mediatek,dsi.txt | 29 +++++ >> > 2 files changed, 160 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt >> > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt >> > >> > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt >> > new file mode 100644 >> > index 0000000..a3811bd >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt >> > @@ -0,0 +1,131 @@ >> > +Mediatek display subsystem >> > +========================== >> > + >> > +The Mediatek display subsystem consists of various DISP function blocks in the >> > +MMSYS register space. The connections between them can be configured by output >> > +and input selectors in the MMSYS_CONFIG register space and register updates can >> > +be synchronized to video frame boundaries with help of a DISP_MUTEX function >> > +block. >> > + >> > +The display-subsystem node binds together all individual device nodes that >> > +comprise the DISP subsystem. >> > + >> > +Required properties: >> > + >> > +- compatible: "mediatek,<chip>-disp" >> >> > +- components: Should contain a list of phandles pointing to the DISP function >> > + block device nodes. >> > +- component-names: Should contain the name of the function block pointed to >> > + by the components phandle of the same index. >> >> NAK. Group these nodes under a parent node, use of-graph or just don't >> put this into DT. Don't invent a new way. > > If I connect the DISP nodes using of-graph bindings, the full graph will > look somewhat like this (including the currently unused function blocks, > all properties but ports and endpoints removed for brevity): > > ovl0@1400c000 { > port { > ovl0_to_color0: endpoint@0 { > remote-endpoint = <&color0_from_ovl0>; > }; > > ovl0_to_wdma0: endpoint@1 { > remote-endpoint = <&wdma0_from_ovl0>; > }; > }; > }; > > ovl1@1400d000 { > port { > ovl1_to_color1: endpoint@0 { > remote-endpoint = <&color1_from_ovl1>; > }; > > ovl1_to_wdma1: endpoint@1 { > remote-endpoint = <&wdma1_from_ovl1>; > }; > }; > }; > > rdma0@1400e000 { > port@0 { > rdma0_from_od: endpoint { > remote-endpoint = <&od_to_rdma0>; > }; > }; > > port@1 { > rdma0_to_ufoe: endpoint@0 { > remote-endpoint = <&ufoe_from_rdma0>; > }; > > rdma0_to_split0: endpoint@1 { > remote-endpoint = <&split0_from_rdma0>; > }; > > rdma0_to_color0: endpoint@2 { > remote-endpoint = <&color0_from_rdma0>; > }; > }; > }; > > rdma1@1400f000 { > port@0 { > rdma1_from_gamma: endpoint { > remote-endpoint = <&gamma_to_rdma1>; > }; > }; > > port@1 { > rdma1_to_dsi0: endpoint@0 { > remote-endpoint = <&dsi0_from_rdma1>; > }; > > rdma1_to_dsi1: endpoint@1 { > remote-endpoint = <&dsi1_from_rdma1>; > }; > > rdma1_to_dpi0: endpoint@2 { > remote-endpoint = <&dpi0_from_rdma1>; > }; > > rdma1_to_color1: endpoint@3 { > remote-endpoint = <&color1_from_rdma1>; > }; > }; > }; > > rdma2@14010000 { > port { > rdma2_to_dsi1: endpoint@0 { > remote-endpoint = <&dsi1_from_rdma2>; > }; > > rdma2_to_dpi0: endpoint@1 { > remote-endpoint = <&dpi0_from_rdma2>; > }; > }; > }; > > wdma0@14011000 { > port { > wdma0_from_ovl0: endpoint@0 { > remote-endpoint = <&ovl0_to_wdma0>; > }; > > wdma0_from_od: endpoint@1 { > remote-endpoint = <&od_to_wdma0>; > }; > > wdma0_from_ufoe: endpoint@2 { > remote-endpoint = <&ufoe_to_wdma0>; > }; > }; > }; > > wdma1@14012000 { > port { > wdma1_from_ovl1: endpoint@0 { > remote-endpoint = <&ovl1_to_wdma1>; > }; > > wdma1_from_gamma: endpoint@1 { > remote-endpoint = <&gamma_to_wdma1>; > }; > }; > }; > > color0@14013000 { > port@0 { > color0_from_rdma0: endpoint@0 { > remote-endpoint = <&rdma0_to_color0>; > }; > > color0_from_ovl0: endpoint@1 { > remote-endpoint = <&ovl0_to_color0>; > }; > }; > > port@1 { > color0_to_aal: endpoint@0 { > remote-endpoint = <&aal_from_color0>; > }; > > color0_to_merge: endpoint@1 { > remote-endpoint = <&merge_from_color0>; > }; > }; > }; > > color1@14014000 { > port@0 { > color1_from_rdma1: endpoint@0 { > remote-endpoint = <&rdma1_to_color1>; > }; > > color1_from_ovl1: endpoint@1 { > remote-endpoint = <&ovl1_to_color1>; > }; > }; > > port@1 { > color1_to_gamma: endpoint@0 { > remote-endpoint = <&gamma_from_color1>; > }; > > color1_to_merge: endpoint@1 { > remote-endpoint = <&merge_from_color1>; > }; > }; > }; > > aal@14015000 { > port@0 { > aal_from_color0: endpoint@0 { > remote-endpoint = <&color0_to_aal>; > }; > > aal_from_merge: endpoint@1 { > remote-endpoint = <&merge_to_aal>; > }; > }; > > port@1 { > aal_to_od: endpoint { > remote-endpoint = <&od_from_aal>; > }; > }; > }; > > gamma@14016000 { > port@0 { > gamma_from_color1: endpoint { > remote-endpoint = <&color1_to_gamma>; > }; > }; > > port@1 { > gamma_to_rdma1: endpoint@0 { > remote-endpoint = <&rdma1_from_gamma>; > }; > > gamma_to_dsi0: endpoint@1 { > remote-endpoint = <&dsi0_from_gamma>; > }; > > gamma_to_dsi1: endpoint@2 { > remote-endpoint = <&dsi1_from_gamma>; > }; > > gamma_to_dpi0: endpoint@3 { > remote-endpoint = <&dpi0_from_gamma>; > }; > > gamma_to_wdma1: endpoint@4 { > remote-endpoint = <&wdma1_from_gamma>; > }; > }; > }; > > merge@14017000 { > port@0 { > merge_from_color0: endpoint { > remote-endpoint = <&color0_to_merge>; > }; > }; > > port@1 { > merge_from_color1: endpoint { > remote-endpoint = <&color1_to_merge>; > }; > }; > > port@2 { > merge_to_aal: endpoint { > remote-endpoint = <&aal_from_merge>; > }; > }; > }; > > split0@14018000 { > port@0 { > split0_from_rdma0: endpoint@0 { > remote-endpoint = <&rdma0_to_split0>; > }; > > split0_from_od: endpoint@1 { > remote-endpoint = <&od_to_split0>; > }; > }; > > port@1 { > split0_to_ufoe: endpoint { > remote-endpoint = <&ufoe_from_split0>; > }; > }; > > port@2 { > split0_to_dsi0: endpoint@0 { > remote-endpoint = <&dsi0_from_split0>; > }; > > split0_to_dsi1: endpoint@1 { > remote-endpoint = <&dsi1_from_split0>; > }; > > split0_to_dpi0: endpoint@2 { > remote-endpoint = <&dpi0_from_split0>; > }; > }; > }; > > split1@14019000 { > port@0 { > split1_from_ufoe: endpoint { > remote-endpoint = <&ufoe_to_split1>; > }; > }; > > port@1 { > split1_to_dsi0: endpoint { > remote-endpoint = <&dsi0_from_split1>; > }; > }; > > port@2 { > split1_to_dsi1: endpoint { > remote-endpoint = <&dsi1_from_split1>; > }; > }; > }; > > ufoe@1401a000 { > port@0 { > ufoe_from_rdma0: endpoint@0 { > remote-endpoint = <&rdma0_to_ufoe>; > }; > > ufoe_from_od: endpoint@1 { > remote-endpoint = <&od_to_ufoe>; > }; > > ufoe_from_split0: endpoint@2 { > remote-endpoint = <&split0_to_ufoe>; > }; > }; > > port@1 { > ufoe_to_dsi0: endpoint@0 { > remote-endpoint = <&dsi0_from_ufoe>; > }; > > ufoe_to_split1: endpoint@1 { > remote-endpoint = <&split1_from_ufoe>; > }; > > ufoe_to_dpi0: endpoint@2 { > remote-endpoint = <&dpi0_from_ufoe>; > }; > > ufoe_to_wdma0: endpoint@3 { > remote-endpoint = <&wdma0_from_ufoe>; > }; > > ufoe_to_dsi1: endpoint@4 { > remote-endpoint = <&dsi1_from_ufoe>; > }; > }; > }; > > dsi0: dsi0@1401b000 { > port@0 { > dsi0_from_ufoe: endpoint@0 { > remote-endpoint = <&ufoe_to_dsi0>; > }; > > dsi0_from_split1: endpoint@1 { > remote-endpoint = <&split1_to_dsi0>; > }; > > dsi0_from_rdma1: endpoint@2 { > remote-endpoint = <&rdma1_to_dsi0>; > }; > > dsi0_from_gamma: endpoint@3 { > remote-endpoint = <&gamma_to_dsi0>; > }; > > dsi0_from_split0: endpoint@4 { > remote-endpoint = <&split0_to_dsi0>; > }; > }; > }; > > dsi1: dsi1@1401c000 { > port@0 { > dsi1_from_split1: endpoint@0 { > remote-endpoint = <&split1_to_dsi1>; > }; > > dsi1_from_rdma1: endpoint@1 { > remote-endpoint = <&rdma1_to_dsi1>; > }; > > dsi1_from_gamma: endpoint@2 { > remote-endpoint = <&gamma_to_dsi1>; > }; > > dsi1_from_split0: endpoint@3 { > remote-endpoint = <&split0_to_dsi1>; > }; > > dsi1_from_rdma2: endpoint@4 { > remote-endpoint = <&rdma2_to_dsi1>; > }; > > dsi1_from_ufoe: endpoint@5 { > remote-endpoint = <&ufoe_to_dsi1>; > }; > }; > }; > > dpi0: dpi0@1401d000 { > port@0 { > dpi0_from_ufoe: endpoint@0 { > remote-endpoint = <&ufoe_to_dpi0>; > }; > > dpi0_from_rdma1: endpoint@1 { > remote-endpoint = <&rdma1_to_dpi0>; > }; > > dpi0_from_gamma: endpoint@2 { > remote-endpoint = <&gamma_to_dpi0>; > }; > > dpi0_from_split0: endpoint@3 { > remote-endpoint = <&split0_to_dpi0>; > }; > > dpi0_from_rdma2: endpoint@4 { > remote-endpoint = <&rdma2_to_dpi0>; > }; > }; > }; > > od@14023000 { > port@0 { > od_from_aal: endpoint { > remote-endpoint = <&aal_to_od>; > }; > }; > > port@1 { > od_to_rdma0: endpoint@0 { > remote-endpoint = <&rdma0_from_od>; > }; > > od_to_ufoe: endpoint@1 { > remote-endpoint = <&ufoe_from_od>; > }; > > od_to_split0: endpoint@2 { > remote-endpoint = <&split0_from_od>; > }; > > od_to_wdma0: endpoint@3 { > remote-endpoint = <&wdma0_from_od>; > }; > }; > }; > > Adding the graph to the mediatek oak-rev3 device tree increases the size > from currently 33 KiB to 41.5 KiB. > As a proponent of the of-graph bindings I certainly like to describe the > hardware connections using DT, but I'm not sure if maybe this is a bit > verbose. We need a graph visualizer... I think there has to be a balance with how much is put into the DT. If the relationship between blocks is pretty fixed then it doesn't really help much to put all this into DT. If every SOC or board can have differing combinations of blocks or connections between blocks then the graph makes sense. Rob
On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Hi Rob, > > thank you for the comments. > > Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring: > [...] >> > +The display-subsystem node binds together all individual device nodes that >> > +comprise the DISP subsystem. >> > + >> > +Required properties: >> > + >> > +- compatible: "mediatek,<chip>-disp" >> >> > +- components: Should contain a list of phandles pointing to the DISP function >> > + block device nodes. >> > +- component-names: Should contain the name of the function block pointed to >> > + by the components phandle of the same index. >> >> NAK. Group these nodes under a parent node, use of-graph or just don't >> put this into DT. Don't invent a new way. > > Ok. The reason I haven't grouped all the display nodes together in the > first place is that they aren't contiguous in the register space. So > instead of: > > ovl@1400c000 { > compatible = "mediatek,mt8173-disp-ovl"; > }; > ... > ufoe@1401a000 { > compatible = "mediatek,mt8173-disp-ufoe"; > }; > > pwm@1401e000 { > compatible = "mediatek,mt8173-disp-pwm"; > }; > > larb@14021000 { > compatible = "mediatek,mt8173-smi-larb"; > }; > > od@14023000 { > compatible = "mediatek,mt8173-disp-od"; > }; > > We'd have: > > display-subsystem@1400c00 { > compatible = "mediatek,mt8173-disp", "simple-bus"; > > ovl@1400c000 { > compatible = "mediatek,mt8173-disp-ovl"; > }; > ... > ufoe@1401a000 { > compatible = "mediatek,mt8173-disp-ufoe"; > }; > > od@14023000 { > compatible = "mediatek,mt8173-disp-od"; > }; > }; > > pwm@1401e000 { > compatible = "mediatek,mt8173-disp-pwm"; > }; > > larb@14021000 { > compatible = "mediatek,mt8173-smi-larb"; > }; > > Note how the display-subsystem node overlaps the larb node. Is that > acceptable? Given what the graph looks like, perhaps. However, do you really need a container node? It only serves to provide a list of nodes (e.g. all the children) to include as components. There are other ways to determine this list. You could find all nodes just searching compatible strings for each component. You just need to bind the drm driver to some other DT node. Is there no node you can pick as the master component? Rob
Am Mittwoch, den 30.09.2015, 11:57 -0500 schrieb Rob Herring: > On Wed, Sep 30, 2015 at 10:30 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Am Freitag, den 18.09.2015, 15:33 -0500 schrieb Rob Herring: > >> On Fri, Sep 18, 2015 at 11:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > >> > From: CK Hu <ck.hu@mediatek.com> > >> > > >> > Add device tree binding documentation for the display subsystem in > >> > Mediatek MT8173 SoCs. > >> > > >> > Signed-off-by: CK Hu <ck.hu@mediatek.com> > >> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > >> > --- > >> > .../bindings/drm/mediatek/mediatek,disp.txt | 131 +++++++++++++++++++++ > >> > .../bindings/drm/mediatek/mediatek,dsi.txt | 29 +++++ > >> > 2 files changed, 160 insertions(+) > >> > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > >> > create mode 100644 Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt > >> > > >> > diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > >> > new file mode 100644 > >> > index 0000000..a3811bd > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt > >> > @@ -0,0 +1,131 @@ > >> > +Mediatek display subsystem > >> > +========================== > >> > + > >> > +The Mediatek display subsystem consists of various DISP function blocks in the > >> > +MMSYS register space. The connections between them can be configured by output > >> > +and input selectors in the MMSYS_CONFIG register space and register updates can > >> > +be synchronized to video frame boundaries with help of a DISP_MUTEX function > >> > +block. > >> > + > >> > +The display-subsystem node binds together all individual device nodes that > >> > +comprise the DISP subsystem. > >> > + > >> > +Required properties: > >> > + > >> > +- compatible: "mediatek,<chip>-disp" > >> > >> > +- components: Should contain a list of phandles pointing to the DISP function > >> > + block device nodes. > >> > +- component-names: Should contain the name of the function block pointed to > >> > + by the components phandle of the same index. > >> > >> NAK. Group these nodes under a parent node, use of-graph or just don't > >> put this into DT. Don't invent a new way. > > > > If I connect the DISP nodes using of-graph bindings, the full graph will > > look somewhat like this (including the currently unused function blocks, > > all properties but ports and endpoints removed for brevity): > > > > ovl0@1400c000 { > > port { > > ovl0_to_color0: endpoint@0 { > > remote-endpoint = <&color0_from_ovl0>; > > }; > > > > ovl0_to_wdma0: endpoint@1 { > > remote-endpoint = <&wdma0_from_ovl0>; > > }; > > }; > > }; > > > > ovl1@1400d000 { > > port { > > ovl1_to_color1: endpoint@0 { > > remote-endpoint = <&color1_from_ovl1>; > > }; > > > > ovl1_to_wdma1: endpoint@1 { > > remote-endpoint = <&wdma1_from_ovl1>; > > }; > > }; > > }; > > > > rdma0@1400e000 { > > port@0 { > > rdma0_from_od: endpoint { > > remote-endpoint = <&od_to_rdma0>; > > }; > > }; > > > > port@1 { > > rdma0_to_ufoe: endpoint@0 { > > remote-endpoint = <&ufoe_from_rdma0>; > > }; > > > > rdma0_to_split0: endpoint@1 { > > remote-endpoint = <&split0_from_rdma0>; > > }; > > > > rdma0_to_color0: endpoint@2 { > > remote-endpoint = <&color0_from_rdma0>; > > }; > > }; > > }; > > > > rdma1@1400f000 { > > port@0 { > > rdma1_from_gamma: endpoint { > > remote-endpoint = <&gamma_to_rdma1>; > > }; > > }; > > > > port@1 { > > rdma1_to_dsi0: endpoint@0 { > > remote-endpoint = <&dsi0_from_rdma1>; > > }; > > > > rdma1_to_dsi1: endpoint@1 { > > remote-endpoint = <&dsi1_from_rdma1>; > > }; > > > > rdma1_to_dpi0: endpoint@2 { > > remote-endpoint = <&dpi0_from_rdma1>; > > }; > > > > rdma1_to_color1: endpoint@3 { > > remote-endpoint = <&color1_from_rdma1>; > > }; > > }; > > }; > > > > rdma2@14010000 { > > port { > > rdma2_to_dsi1: endpoint@0 { > > remote-endpoint = <&dsi1_from_rdma2>; > > }; > > > > rdma2_to_dpi0: endpoint@1 { > > remote-endpoint = <&dpi0_from_rdma2>; > > }; > > }; > > }; > > > > wdma0@14011000 { > > port { > > wdma0_from_ovl0: endpoint@0 { > > remote-endpoint = <&ovl0_to_wdma0>; > > }; > > > > wdma0_from_od: endpoint@1 { > > remote-endpoint = <&od_to_wdma0>; > > }; > > > > wdma0_from_ufoe: endpoint@2 { > > remote-endpoint = <&ufoe_to_wdma0>; > > }; > > }; > > }; > > > > wdma1@14012000 { > > port { > > wdma1_from_ovl1: endpoint@0 { > > remote-endpoint = <&ovl1_to_wdma1>; > > }; > > > > wdma1_from_gamma: endpoint@1 { > > remote-endpoint = <&gamma_to_wdma1>; > > }; > > }; > > }; > > > > color0@14013000 { > > port@0 { > > color0_from_rdma0: endpoint@0 { > > remote-endpoint = <&rdma0_to_color0>; > > }; > > > > color0_from_ovl0: endpoint@1 { > > remote-endpoint = <&ovl0_to_color0>; > > }; > > }; > > > > port@1 { > > color0_to_aal: endpoint@0 { > > remote-endpoint = <&aal_from_color0>; > > }; > > > > color0_to_merge: endpoint@1 { > > remote-endpoint = <&merge_from_color0>; > > }; > > }; > > }; > > > > color1@14014000 { > > port@0 { > > color1_from_rdma1: endpoint@0 { > > remote-endpoint = <&rdma1_to_color1>; > > }; > > > > color1_from_ovl1: endpoint@1 { > > remote-endpoint = <&ovl1_to_color1>; > > }; > > }; > > > > port@1 { > > color1_to_gamma: endpoint@0 { > > remote-endpoint = <&gamma_from_color1>; > > }; > > > > color1_to_merge: endpoint@1 { > > remote-endpoint = <&merge_from_color1>; > > }; > > }; > > }; > > > > aal@14015000 { > > port@0 { > > aal_from_color0: endpoint@0 { > > remote-endpoint = <&color0_to_aal>; > > }; > > > > aal_from_merge: endpoint@1 { > > remote-endpoint = <&merge_to_aal>; > > }; > > }; > > > > port@1 { > > aal_to_od: endpoint { > > remote-endpoint = <&od_from_aal>; > > }; > > }; > > }; > > > > gamma@14016000 { > > port@0 { > > gamma_from_color1: endpoint { > > remote-endpoint = <&color1_to_gamma>; > > }; > > }; > > > > port@1 { > > gamma_to_rdma1: endpoint@0 { > > remote-endpoint = <&rdma1_from_gamma>; > > }; > > > > gamma_to_dsi0: endpoint@1 { > > remote-endpoint = <&dsi0_from_gamma>; > > }; > > > > gamma_to_dsi1: endpoint@2 { > > remote-endpoint = <&dsi1_from_gamma>; > > }; > > > > gamma_to_dpi0: endpoint@3 { > > remote-endpoint = <&dpi0_from_gamma>; > > }; > > > > gamma_to_wdma1: endpoint@4 { > > remote-endpoint = <&wdma1_from_gamma>; > > }; > > }; > > }; > > > > merge@14017000 { > > port@0 { > > merge_from_color0: endpoint { > > remote-endpoint = <&color0_to_merge>; > > }; > > }; > > > > port@1 { > > merge_from_color1: endpoint { > > remote-endpoint = <&color1_to_merge>; > > }; > > }; > > > > port@2 { > > merge_to_aal: endpoint { > > remote-endpoint = <&aal_from_merge>; > > }; > > }; > > }; > > > > split0@14018000 { > > port@0 { > > split0_from_rdma0: endpoint@0 { > > remote-endpoint = <&rdma0_to_split0>; > > }; > > > > split0_from_od: endpoint@1 { > > remote-endpoint = <&od_to_split0>; > > }; > > }; > > > > port@1 { > > split0_to_ufoe: endpoint { > > remote-endpoint = <&ufoe_from_split0>; > > }; > > }; > > > > port@2 { > > split0_to_dsi0: endpoint@0 { > > remote-endpoint = <&dsi0_from_split0>; > > }; > > > > split0_to_dsi1: endpoint@1 { > > remote-endpoint = <&dsi1_from_split0>; > > }; > > > > split0_to_dpi0: endpoint@2 { > > remote-endpoint = <&dpi0_from_split0>; > > }; > > }; > > }; > > > > split1@14019000 { > > port@0 { > > split1_from_ufoe: endpoint { > > remote-endpoint = <&ufoe_to_split1>; > > }; > > }; > > > > port@1 { > > split1_to_dsi0: endpoint { > > remote-endpoint = <&dsi0_from_split1>; > > }; > > }; > > > > port@2 { > > split1_to_dsi1: endpoint { > > remote-endpoint = <&dsi1_from_split1>; > > }; > > }; > > }; > > > > ufoe@1401a000 { > > port@0 { > > ufoe_from_rdma0: endpoint@0 { > > remote-endpoint = <&rdma0_to_ufoe>; > > }; > > > > ufoe_from_od: endpoint@1 { > > remote-endpoint = <&od_to_ufoe>; > > }; > > > > ufoe_from_split0: endpoint@2 { > > remote-endpoint = <&split0_to_ufoe>; > > }; > > }; > > > > port@1 { > > ufoe_to_dsi0: endpoint@0 { > > remote-endpoint = <&dsi0_from_ufoe>; > > }; > > > > ufoe_to_split1: endpoint@1 { > > remote-endpoint = <&split1_from_ufoe>; > > }; > > > > ufoe_to_dpi0: endpoint@2 { > > remote-endpoint = <&dpi0_from_ufoe>; > > }; > > > > ufoe_to_wdma0: endpoint@3 { > > remote-endpoint = <&wdma0_from_ufoe>; > > }; > > > > ufoe_to_dsi1: endpoint@4 { > > remote-endpoint = <&dsi1_from_ufoe>; > > }; > > }; > > }; > > > > dsi0: dsi0@1401b000 { > > port@0 { > > dsi0_from_ufoe: endpoint@0 { > > remote-endpoint = <&ufoe_to_dsi0>; > > }; > > > > dsi0_from_split1: endpoint@1 { > > remote-endpoint = <&split1_to_dsi0>; > > }; > > > > dsi0_from_rdma1: endpoint@2 { > > remote-endpoint = <&rdma1_to_dsi0>; > > }; > > > > dsi0_from_gamma: endpoint@3 { > > remote-endpoint = <&gamma_to_dsi0>; > > }; > > > > dsi0_from_split0: endpoint@4 { > > remote-endpoint = <&split0_to_dsi0>; > > }; > > }; > > }; > > > > dsi1: dsi1@1401c000 { > > port@0 { > > dsi1_from_split1: endpoint@0 { > > remote-endpoint = <&split1_to_dsi1>; > > }; > > > > dsi1_from_rdma1: endpoint@1 { > > remote-endpoint = <&rdma1_to_dsi1>; > > }; > > > > dsi1_from_gamma: endpoint@2 { > > remote-endpoint = <&gamma_to_dsi1>; > > }; > > > > dsi1_from_split0: endpoint@3 { > > remote-endpoint = <&split0_to_dsi1>; > > }; > > > > dsi1_from_rdma2: endpoint@4 { > > remote-endpoint = <&rdma2_to_dsi1>; > > }; > > > > dsi1_from_ufoe: endpoint@5 { > > remote-endpoint = <&ufoe_to_dsi1>; > > }; > > }; > > }; > > > > dpi0: dpi0@1401d000 { > > port@0 { > > dpi0_from_ufoe: endpoint@0 { > > remote-endpoint = <&ufoe_to_dpi0>; > > }; > > > > dpi0_from_rdma1: endpoint@1 { > > remote-endpoint = <&rdma1_to_dpi0>; > > }; > > > > dpi0_from_gamma: endpoint@2 { > > remote-endpoint = <&gamma_to_dpi0>; > > }; > > > > dpi0_from_split0: endpoint@3 { > > remote-endpoint = <&split0_to_dpi0>; > > }; > > > > dpi0_from_rdma2: endpoint@4 { > > remote-endpoint = <&rdma2_to_dpi0>; > > }; > > }; > > }; > > > > od@14023000 { > > port@0 { > > od_from_aal: endpoint { > > remote-endpoint = <&aal_to_od>; > > }; > > }; > > > > port@1 { > > od_to_rdma0: endpoint@0 { > > remote-endpoint = <&rdma0_from_od>; > > }; > > > > od_to_ufoe: endpoint@1 { > > remote-endpoint = <&ufoe_from_od>; > > }; > > > > od_to_split0: endpoint@2 { > > remote-endpoint = <&split0_from_od>; > > }; > > > > od_to_wdma0: endpoint@3 { > > remote-endpoint = <&wdma0_from_od>; > > }; > > }; > > }; > > > > Adding the graph to the mediatek oak-rev3 device tree increases the size > > from currently 33 KiB to 41.5 KiB. > > As a proponent of the of-graph bindings I certainly like to describe the > > hardware connections using DT, but I'm not sure if maybe this is a bit > > verbose. > > We need a graph visualizer... Yes :) > I think there has to be a balance with how much is put into the DT. If > the relationship between blocks is pretty fixed then it doesn't really > help much to put all this into DT. If every SOC or board can have > differing combinations of blocks or connections between blocks then > the graph makes sense. I expect the graph to be constant for a given SoC like MT8173. regards Philipp
Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring: > On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Note how the display-subsystem node overlaps the larb node. Is that > > acceptable? > > Given what the graph looks like, perhaps. However, do you really need > a container node? It only serves to provide a list of nodes (e.g. all > the children) to include as components. There are other ways to > determine this list. You could find all nodes just searching > compatible strings for each component. You just need to bind the drm > driver to some other DT node. Is there no node you can pick as the > master component? There is the mmsys clock-controller node at the top of the MMSYS address space (0x14000000-0x14ffffff): mmsys: clock-controller@14000000 { compatible = "mediatek,mt8173-mmsys", "syscon"; reg = <0 0x14000000 0 0x1000>; #clock-cells = <1>; }; Its register space also contains the MMSYS_CONFIG region that controls the multiplexers between the display function blocks, so that would be a good candidate. No driver binds to this node yet, the clocks are registered with CLK_OF_DECLARE. I'll try to bind to this node and have the driver find sibling nodes using their compatible strings. thanks Philipp
On Thu, Oct 1, 2015 at 3:59 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring: >> On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> > Note how the display-subsystem node overlaps the larb node. Is that >> > acceptable? >> >> Given what the graph looks like, perhaps. However, do you really need >> a container node? It only serves to provide a list of nodes (e.g. all >> the children) to include as components. There are other ways to >> determine this list. You could find all nodes just searching >> compatible strings for each component. You just need to bind the drm >> driver to some other DT node. Is there no node you can pick as the >> master component? > > There is the mmsys clock-controller node at the top of the MMSYS address > space (0x14000000-0x14ffffff): > > mmsys: clock-controller@14000000 { > compatible = "mediatek,mt8173-mmsys", "syscon"; > reg = <0 0x14000000 0 0x1000>; > #clock-cells = <1>; > }; > > Its register space also contains the MMSYS_CONFIG region that controls > the multiplexers between the display function blocks, so that would be a > good candidate. > No driver binds to this node yet, the clocks are registered with > CLK_OF_DECLARE. > > I'll try to bind to this node and have the driver find sibling nodes > using their compatible strings. That doesn't seem like a good choice since there are other functions in the block. I was thinking one of the display related blocks like whatever block provides the main crtc functions. Rob
On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh@kernel.org> wrote: > On Thu, Oct 1, 2015 at 3:59 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring: >>> On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >>> > Note how the display-subsystem node overlaps the larb node. Is that >>> > acceptable? >>> >>> Given what the graph looks like, perhaps. However, do you really need >>> a container node? It only serves to provide a list of nodes (e.g. all >>> the children) to include as components. There are other ways to >>> determine this list. You could find all nodes just searching >>> compatible strings for each component. You just need to bind the drm >>> driver to some other DT node. Is there no node you can pick as the >>> master component? >> >> There is the mmsys clock-controller node at the top of the MMSYS address >> space (0x14000000-0x14ffffff): >> >> mmsys: clock-controller@14000000 { >> compatible = "mediatek,mt8173-mmsys", "syscon"; >> reg = <0 0x14000000 0 0x1000>; >> #clock-cells = <1>; >> }; >> >> Its register space also contains the MMSYS_CONFIG region that controls >> the multiplexers between the display function blocks, so that would be a >> good candidate. >> No driver binds to this node yet, the clocks are registered with >> CLK_OF_DECLARE. >> >> I'll try to bind to this node and have the driver find sibling nodes >> using their compatible strings. > > That doesn't seem like a good choice since there are other functions > in the block. I was thinking one of the display related blocks like > whatever block provides the main crtc functions. The two "OVL" nodes correspond to the "crtc" functions of the two display pipes in this SoC, we would setup the display-subsystem node like this: display-subsystem { compatible = "mediatek,display-subsystem"; ports = <&ovl0>, <&ovl1>; }; And, any node that needs to poke around in the mmsys_config area, like an ovl, could access it using a phandle <&mmsys> to the common "mediatek,mt8173-mmsys" device, like this: ovl0: ovl@1400c000 { compatible = "mediatek,mt8173-ovl"; reg = <0 0x1400c000 0 0x1000>; interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>; clocks = <&mmsys CLK_MM_DISP_OVL0>; iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>; mediatek,mmsys = <&mmsys>; status = "disabled"; }; One idea to reduce the size of the of_graph would be to just model the entrance/exit points from the MM subsystem in dt. So, instead of: ovl0 -> color0 -> aal -> od -> ufoe -> dsi0 -> dsi-edp-bridge ovl1 -> color1 -> gamma0 -> dpi0 -> hdmi We can do something like this: ovl0 -> dsi0 -> dsi-edp-bridge ovl1 -> dpi0 -> hdmi But ... this might be too limiting for some applications. So, I think we should just live with the big graph. I would, however, recommend that you move the graph to its own mt8173-display-graph.dtsi file which would be #included into a board specific .dts file. If you let the board .dts include the graph, then we can possibly shrink the resulting board specific device tree - At first we would make the full graph in one .dtsi as above "mt8173-display-graph.dtsi". This is the full reference graph that will always work. Then, if a particular board used a fixed configuration, the system integrator could create a smaller graph with just the graph nodes that they care about. For example, if mt8173-evb only supports a single HDMI output, then perhaps you would just set up the endpoints for this single fixed function display pipe in mt8173-evb-display-graph.dtsi: &ovl1 { status = "okay"; port { ovl1_to_color1: endpoint@0 { remote-endpoint = <&color1_from_ovl1>; }; }; }; &color1 { status = "okay"; port@0 { color1_from_ovl1: endpoint@1 { remote-endpoint = <&ovl1_to_color1>; }; }; port@1 { color1_to_gamma: endpoint@0 { remote-endpoint = <&gamma_from_color1>; }; }; }; &gamma { status = "okay"; port@0 { gamma_from_color1: endpoint { remote-endpoint = <&color1_to_gamma>; }; }; port@1 { gamma_to_dpi0: endpoint@3 { remote-endpoint = <&dpi0_from_gamma>; }; }; }; &dpi0 { status = "okay"; port@0 { dpi0_from_gamma: endpoint@2 { remote-endpoint = <&gamma_to_dpi0>; }; }; port@1 { dpi0_to_hdmi: endpoint { remote-endpoint = <&hdmi_from_dpi0>; }; }; }; Notice that this also only enables the nodes that we care about (all of the other nodes would be disabled by default). I this would allow full flexibility but also drastically cut down on the resulting .dts size. The individual component drivers would then march down their graphs ensuring that all of the corresponding components have been bound, and then themselves appropriately based on their set of endpoints that are available. What do you think? -Dan > > Rob > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Am Donnerstag, den 01.10.2015, 07:58 -0500 schrieb Rob Herring: > On Thu, Oct 1, 2015 at 3:59 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring: > >> On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > >> > Note how the display-subsystem node overlaps the larb node. Is that > >> > acceptable? > >> > >> Given what the graph looks like, perhaps. However, do you really need > >> a container node? It only serves to provide a list of nodes (e.g. all > >> the children) to include as components. There are other ways to > >> determine this list. You could find all nodes just searching > >> compatible strings for each component. You just need to bind the drm > >> driver to some other DT node. Is there no node you can pick as the > >> master component? > > > > There is the mmsys clock-controller node at the top of the MMSYS address > > space (0x14000000-0x14ffffff): > > > > mmsys: clock-controller@14000000 { > > compatible = "mediatek,mt8173-mmsys", "syscon"; > > reg = <0 0x14000000 0 0x1000>; > > #clock-cells = <1>; > > }; > > > > Its register space also contains the MMSYS_CONFIG region that controls > > the multiplexers between the display function blocks, so that would be a > > good candidate. > > No driver binds to this node yet, the clocks are registered with > > CLK_OF_DECLARE. > > > > I'll try to bind to this node and have the driver find sibling nodes > > using their compatible strings. > > That doesn't seem like a good choice since there are other functions > in the block. I was thinking one of the display related blocks like > whatever block provides the main crtc functions. Why exactly isn't this a good choice? Since all display function blocks are spread throughout the MMSYS address space, using the mmsys driver to create the DRM/component master platform driver that collects its subdevices sounded logical. Another candidate would be the mutex node, which can synchronize the configuration updates to the function blocks in the MMSYS region to frame borders. The hardware blocks that most closely correspond to crtc functionality are the two OVL blocks, as Daniel points out. They provide DMA and plane composition. But they are equivalent, and there is no single central node, same as with the two IPUs on i.MX6. Also, I'd like to keep the possibility of not tying OVL blocks to crtc functionality too closely, because there are three more separate RDMA units, at least one of which could be used to implement a third pipe in the future. Either way, if we go this path (one driver scanning DT for known component driver compatible values), whichever node the master driver binds to is a Linux implementation detail, and it should be completely hidden from the device tree and binding specs. regards Philipp
Am Donnerstag, den 01.10.2015, 22:29 +0800 schrieb Daniel Kurtz: > On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh@kernel.org> wrote: > > I was thinking one of the display related blocks like > > whatever block provides the main crtc functions. > > The two "OVL" nodes correspond to the "crtc" functions of the two > display pipes in this SoC, we would setup the display-subsystem node > like this: > > display-subsystem { > compatible = "mediatek,display-subsystem"; Yes, the problem with the ovl nodes is that there are two equivalent ones. Having two equivalent ipu nodes on i.MX6 was the reason to introduce the display-subsystem node in the first place, but I'd very much prefer to avoid it, if possible. [...] > And, any node that needs to poke around in the mmsys_config area, like > an ovl, could access it using a phandle <&mmsys> to the common > "mediatek,mt8173-mmsys" device, like this: > > ovl0: ovl@1400c000 { > compatible = "mediatek,mt8173-ovl"; > reg = <0 0x1400c000 0 0x1000>; > interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>; > clocks = <&mmsys CLK_MM_DISP_OVL0>; > iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>; > mediatek,mmsys = <&mmsys>; > status = "disabled"; > }; I think the ovl node has no business linking to mmsys_config. It's the drm driver code that sets up the pipeline, including the crtc -> encoder connections > One idea to reduce the size of the of_graph would be to just model the > entrance/exit points from the MM subsystem in dt. So, instead of: > ovl0 -> color0 -> aal -> od -> ufoe -> dsi0 -> dsi-edp-bridge > ovl1 -> color1 -> gamma0 -> dpi0 -> hdmi > > We can do something like this: > ovl0 -> dsi0 -> dsi-edp-bridge > ovl1 -> dpi0 -> hdmi > > But ... this might be too limiting for some applications. I'm already worried about having left out the DISP_PATH0 and DISP_PATH1 multiplexers from my DT graph example. There are so many ways to do this slightly wrong and then having to live with the stable bindings, that I'd rather not put an approximation of the real hardware into the device tree. Now that I've heard it is acceptable for one driver to look for its components by their compatible value, I'm leaning towards not describing the graph in the DT. > So, I think we should just live with the big graph. > I would, however, recommend that you move the graph to its own > mt8173-display-graph.dtsi file which would be #included into a board > specific .dts file. Separating the graph out into its own include file would help to make the main dtsi more readable, though. > If you let the board .dts include the graph, then we can possibly > shrink the resulting board specific device tree - > At first we would make the full graph in one .dtsi as above > "mt8173-display-graph.dtsi". > This is the full reference graph that will always work. > > Then, if a particular board used a fixed configuration, the system > integrator could create a smaller graph with just the graph nodes that > they care about. > > For example, if mt8173-evb only supports a single HDMI output, then > perhaps you would just > set up the endpoints for this single fixed function display pipe in > mt8173-evb-display-graph.dtsi: [...] Especially for development boards I'd rather keep all possibilites. Sure, if nothing is connected to the DSIs, disable them and drop thier possible connections, but I'd still keep ovl0 for example, because it could be connected to hdmi, even though the driver doesn't support it. I realize this was probably just an example, so yes, I think this is a valid method of reducing device tree size for a given board if necessary. regards Philipp
On Fri, Oct 2, 2015 at 3:40 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Donnerstag, den 01.10.2015, 22:29 +0800 schrieb Daniel Kurtz: >> On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh@kernel.org> wrote: >> > I was thinking one of the display related blocks like >> > whatever block provides the main crtc functions. >> >> The two "OVL" nodes correspond to the "crtc" functions of the two >> display pipes in this SoC, we would setup the display-subsystem node >> like this: >> >> display-subsystem { >> compatible = "mediatek,display-subsystem"; > > Yes, the problem with the ovl nodes is that there are two equivalent > ones. Having two equivalent ipu nodes on i.MX6 was the reason to > introduce the display-subsystem node in the first place, but I'd very > much prefer to avoid it, if possible. Oh, interesting. How is it a problem that there are two equivalent ovl nodes? Why do you prefer to avoid the display-subsystem node? BTW, what I really meant was for my example was that the 'display-subsystem' 'ports' node lists phandles representing the 'root' nodes for the for the display-subsystem graph. This matches what we did for rk3288.dtsi: display-subsystem { compatible = "mediatek,display-subsystem"; ports = <&ovl0_out>, <&ovl1_out>; }; > [...] >> And, any node that needs to poke around in the mmsys_config area, like >> an ovl, could access it using a phandle <&mmsys> to the common >> "mediatek,mt8173-mmsys" device, like this: >> >> ovl0: ovl@1400c000 { >> compatible = "mediatek,mt8173-ovl"; >> reg = <0 0x1400c000 0 0x1000>; >> interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>; >> clocks = <&mmsys CLK_MM_DISP_OVL0>; >> iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>; >> mediatek,mmsys = <&mmsys>; >> status = "disabled"; >> }; > > I think the ovl node has no business linking to mmsys_config. > It's the drm driver code that sets up the pipeline, including the crtc > -> encoder connections My original thinking is that the "ovl" node should link to the mmsys_config since the mmsys_config has the ovl output select registers. An 'ovl' driver would then use the phandle to the syscon to find a regmap to safely access the registers it needs. >> One idea to reduce the size of the of_graph would be to just model the >> entrance/exit points from the MM subsystem in dt. So, instead of: >> ovl0 -> color0 -> aal -> od -> ufoe -> dsi0 -> dsi-edp-bridge >> ovl1 -> color1 -> gamma0 -> dpi0 -> hdmi >> >> We can do something like this: >> ovl0 -> dsi0 -> dsi-edp-bridge >> ovl1 -> dpi0 -> hdmi >> >> But ... this might be too limiting for some applications. > > I'm already worried about having left out the DISP_PATH0 and DISP_PATH1 > multiplexers from my DT graph example. There are so many ways to do this > slightly wrong and then having to live with the stable bindings, that > I'd rather not put an approximation of the real hardware into the device > tree. > Now that I've heard it is acceptable for one driver to look for its > components by their compatible value, I'm leaning towards not describing > the graph in the DT. Sure, it would be interesting to see how that would look, too. -Dan
On Fri, Oct 2, 2015 at 2:18 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Am Donnerstag, den 01.10.2015, 07:58 -0500 schrieb Rob Herring: >> On Thu, Oct 1, 2015 at 3:59 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> > Am Mittwoch, den 30.09.2015, 12:13 -0500 schrieb Rob Herring: >> >> On Mon, Sep 21, 2015 at 3:11 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote: >> >> > Note how the display-subsystem node overlaps the larb node. Is that >> >> > acceptable? >> >> >> >> Given what the graph looks like, perhaps. However, do you really need >> >> a container node? It only serves to provide a list of nodes (e.g. all >> >> the children) to include as components. There are other ways to >> >> determine this list. You could find all nodes just searching >> >> compatible strings for each component. You just need to bind the drm >> >> driver to some other DT node. Is there no node you can pick as the >> >> master component? >> > >> > There is the mmsys clock-controller node at the top of the MMSYS address >> > space (0x14000000-0x14ffffff): >> > >> > mmsys: clock-controller@14000000 { >> > compatible = "mediatek,mt8173-mmsys", "syscon"; >> > reg = <0 0x14000000 0 0x1000>; >> > #clock-cells = <1>; >> > }; >> > >> > Its register space also contains the MMSYS_CONFIG region that controls >> > the multiplexers between the display function blocks, so that would be a >> > good candidate. >> > No driver binds to this node yet, the clocks are registered with >> > CLK_OF_DECLARE. >> > >> > I'll try to bind to this node and have the driver find sibling nodes >> > using their compatible strings. >> >> That doesn't seem like a good choice since there are other functions >> in the block. I was thinking one of the display related blocks like >> whatever block provides the main crtc functions. > > Why exactly isn't this a good choice? Since all display function blocks > are spread throughout the MMSYS address space, using the mmsys driver to > create the DRM/component master platform driver that collects its > subdevices sounded logical. Given it is a syscon, I was assuming there are other unrelated functions. If everything in it is display related, then it would be a fine choice. > Another candidate would be the mutex node, which can synchronize the > configuration updates to the function blocks in the MMSYS region to > frame borders. > > The hardware blocks that most closely correspond to crtc functionality > are the two OVL blocks, as Daniel points out. They provide DMA and plane > composition. But they are equivalent, and there is no single central > node, same as with the two IPUs on i.MX6. Just curious, are 2 IPUs represented as 1 or 2 struct drm_device? > Also, I'd like to keep the possibility of not tying OVL blocks to crtc > functionality too closely, because there are three more separate RDMA > units, at least one of which could be used to implement a third pipe in > the future. > > Either way, if we go this path (one driver scanning DT for known > component driver compatible values), whichever node the master driver > binds to is a Linux implementation detail, and it should be completely > hidden from the device tree and binding specs. Agreed. Rob
Hi Daniel, Am Freitag, den 02.10.2015, 21:47 +0800 schrieb Daniel Kurtz: > On Fri, Oct 2, 2015 at 3:40 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote: > > Am Donnerstag, den 01.10.2015, 22:29 +0800 schrieb Daniel Kurtz: > >> On Thu, Oct 1, 2015 at 8:58 PM, Rob Herring <robh@kernel.org> wrote: > >> > I was thinking one of the display related blocks like > >> > whatever block provides the main crtc functions. > >> > >> The two "OVL" nodes correspond to the "crtc" functions of the two > >> display pipes in this SoC, we would setup the display-subsystem node > >> like this: > >> > >> display-subsystem { > >> compatible = "mediatek,display-subsystem"; > > > > Yes, the problem with the ovl nodes is that there are two equivalent > > ones. Having two equivalent ipu nodes on i.MX6 was the reason to > > introduce the display-subsystem node in the first place, but I'd very > > much prefer to avoid it, if possible. > > Oh, interesting. How is it a problem that there are two equivalent ovl nodes? Which one of the two should create the drm device? > Why do you prefer to avoid the display-subsystem node? Because then we don't have to argue about what it should look like ;) Mostly because nodes like this don't describe an actual hardware devices, but rather how the hardware designers intended other, actually existing hardware devices to work together (in the best case), or just how the Linux driver developers wanted to use the components together (in the worst case). If the display-subsystem node is necessary or useful, I won't argue against it. If we can do without, I'll be happier. > BTW, what I really meant was for my example was that the > 'display-subsystem' 'ports' node lists phandles representing the > 'root' nodes for the for the display-subsystem graph. > This matches what we did for rk3288.dtsi: > > display-subsystem { > compatible = "mediatek,display-subsystem"; > ports = <&ovl0_out>, <&ovl1_out>; > }; Oh, ok then. I'm worried again that we would be describing what the driver should load, not what the hardware looks like. Note that the ovl_out ports have no significance outside of the display subsystem. In the RK3288 and i.MX6 cases, the 'external' ports of the display core are listed, to which the encoders are connected. On MT8173 this would correspond to: display-subsystem { compatible = "mediatek,display-subsystem"; ports = <&dsi0_out>, <&dsi1_out>, <&dpi0_out>; }; [...] > > I think the ovl node has no business linking to mmsys_config. > > It's the drm driver code that sets up the pipeline, including the crtc > > -> encoder connections > > My original thinking is that the "ovl" node should link to the > mmsys_config since the mmsys_config has the ovl output select > registers. I see. If each function block takes care of the attached SEL input selectors and SOUT/MOUT output selectors, wouldn't all of them have to link to the mmsys_config node, then? That's certainly a possibility, but in this case I feel like having a single, central instance to handle building the paths and arbitrating register access with help of the disp_mutex would be easier to understand. regards Philipp
Hi Rob, Am Freitag, den 02.10.2015, 09:24 -0500 schrieb Rob Herring: [...] > >> > I'll try to bind to this node and have the driver find sibling nodes > >> > using their compatible strings. > >> > >> That doesn't seem like a good choice since there are other functions > >> in the block. I was thinking one of the display related blocks like > >> whatever block provides the main crtc functions. > > > > Why exactly isn't this a good choice? Since all display function blocks > > are spread throughout the MMSYS address space, using the mmsys driver to > > create the DRM/component master platform driver that collects its > > subdevices sounded logical. > > Given it is a syscon, I was assuming there are other unrelated > functions. If everything in it is display related, then it would be a > fine choice. Oh, ok. Next to the display blocks there's also a similar graph of MDP ("multimedia data path") blocks that can perform memory-to-memory scaling and rotation that use the same multiplexers and mutex, and the HDMI encoder controls the FIFO between its parallel output and the HDMI PHY via (separate) registers in the MMSYS_CONFIG region. Still, I think these are sufficiently display related. > > Another candidate would be the mutex node, which can synchronize the > > configuration updates to the function blocks in the MMSYS region to > > frame borders. > > > > The hardware blocks that most closely correspond to crtc functionality > > are the two OVL blocks, as Daniel points out. They provide DMA and plane > > composition. But they are equivalent, and there is no single central > > node, same as with the two IPUs on i.MX6. > > Just curious, are 2 IPUs represented as 1 or 2 struct drm_device? One. There's bus multiplexer fabric between the IPU outputs and the LVDS/HDMI encoders, so it's all connected. regards Philipp
diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt new file mode 100644 index 0000000..a3811bd --- /dev/null +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,disp.txt @@ -0,0 +1,131 @@ +Mediatek display subsystem +========================== + +The Mediatek display subsystem consists of various DISP function blocks in the +MMSYS register space. The connections between them can be configured by output +and input selectors in the MMSYS_CONFIG register space and register updates can +be synchronized to video frame boundaries with help of a DISP_MUTEX function +block. + +The display-subsystem node binds together all individual device nodes that +comprise the DISP subsystem. + +Required properties: + +- compatible: "mediatek,<chip>-disp" +- components: Should contain a list of phandles pointing to the DISP function + block device nodes. +- component-names: Should contain the name of the function block pointed to + by the components phandle of the same index. +- mmsys-config: Should contain a phandle pointing to the MMSYS node. +- disp-mutex: Should contain a phandle pointing to the DISP_MUTEX node. + +Example: + +display-subsystem { + compatible = "mediatek,mt8173-disp"; + components = <&ovl0>, <&rdma0>, <&color0>, <&aal>, + <&ufoe>, <&dsi0>, <&od>; + component-names = "ovl0", "rdma0", "color0", "aal", + "ufoe", "dsi0", "od"; + mmsys-config = <&mmsys>; + disp-mutex = <&mutex>; +}; + +DISP function blocks +==================== + +A display stream starts at a source function block that reads pixel data from +memory and ends with a sink function block that drives pixels on a display +interface, or writes pixels back to memory. All DISP function blocks have +their own register space, interrupt, and clock gate. The blocks that can +access memory additionally have to list the IOMMU and local arbiter they are +connected to. + +For a description of the display interface sink function blocks, see +Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt + +Required properties (all function blocks): +- compatible: "mediatek,<chip>-disp-<function>", one of + "mediatek,<chip>-disp-ovl" - overlay (4 layers, blending, csc) + "mediatek,<chip>-disp-rdma" - read DMA / line buffer + "mediatek,<chip>-disp-color" - color processor + "mediatek,<chip>-disp-aal" - adaptive ambient light controller + "mediatek,<chip>-disp-gamma" - gamma correction + "mediatek,<chip>-disp-ufoe" - data compression engine + "mediatek,<chip>-disp-mutex" - display mutex + "mediatek,<chip>-disp-od" - overdrive +- reg: Physical base address and length of the function block register space +- interrupts: The interrupt signal from the function block. +- clocks: device clocks + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. +- compatible: "mediatek,<chip>-ddp" +- power-domains: a phandle to DDP power domain node. + +Required properties (DMA function blocks): +- compatible: Should be one of + "mediatek,<chip>-disp-ovl" + "mediatek,<chip>-disp-rdma" +- larb: Should a phandles pointing to the local arbiter device as defined in + Documentation/devicetree/bindings/soc/mediatek/mediatek,smi-larb.txt +- iommus: required a iommu node + +Examples: + +ovl0: ovl@1400c000 { + compatible = "mediatek,mt8173-disp-ovl"; + reg = <0 0x1400c000 0 0x1000>; + interrupts = <GIC_SPI 180 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_OVL0>; + iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_OVL0>; +}; + +rdma0: rdma@1400e000 { + compatible = "mediatek,mt8173-disp-rdma"; + reg = <0 0x1400e000 0 0x1000>; + interrupts = <GIC_SPI 182 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_RDMA0>; + iommus = <&iommu M4U_LARB0_ID M4U_PORT_DISP_RDMA0>; +}; + +color0: color@14013000 { + compatible = "mediatek,mt8173-disp-color"; + reg = <0 0x14013000 0 0x1000>; + interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_COLOR0>; +}; + +aal: aal@14015000 { + compatible = "mediatek,mt8173-disp-aal"; + reg = <0 0x14015000 0 0x1000>; + interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_AAL>; +}; + +gamma: gamma@14016000 { + compatible = "mediatek,mt8173-disp-gamma"; + reg = <0 0x14016000 0 0x1000>; + interrupts = <GIC_SPI 190 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_GAMMA>; +}; + +ufoe: ufoe@1401a000 { + compatible = "mediatek,mt8173-disp-ufoe"; + reg = <0 0x1401a000 0 0x1000>; + interrupts = <GIC_SPI 191 IRQ_TYPE_LEVEL_LOW>; + clocks = <&mmsys CLK_MM_DISP_UFOE>; +}; + +mutex: mutex@14020000 { + compatible = "mediatek,mt8173-disp-mutex"; + reg = <0 0x14020000 0 0x1000>; + interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_LOW>; + power-domains = <&scpsys MT8173_POWER_DOMAIN_MM>; + clocks = <&mmsys CLK_MM_MUTEX_32K>; +}; + +od: od@14023000 { + compatible = "mediatek,mt8173-disp-od"; + reg = <0 0x14023000 0 0x1000>; + clocks = <&mmsys CLK_MM_DISP_OD>; +}; diff --git a/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt new file mode 100644 index 0000000..e892ef1 --- /dev/null +++ b/Documentation/devicetree/bindings/drm/mediatek/mediatek,dsi.txt @@ -0,0 +1,29 @@ +Mediatek DSI Device +=================== + +The Mediatek DSI function block is a sink of the display subsystem and can +drive up to 4-lane MIPI DSI output. Two DSIs can be synchronized for dual- +channel output. + +Required properties: +- compatible: "mediatek,<chip>-dsi" +- reg: Physical base address and length of the controller's registers +- clocks: device clocks + See Documentation/devicetree/bindings/clock/clock-bindings.txt for details. +- clock-names: must contain "engine" and "digital". + +Example: + +dsi0: dsi@1401b000 { + compatible = "mediatek,mt8173-dsi"; + reg = <0 0x1401b000 0 0x1000>, /* DSI0 */ + <0 0x10215000 0 0x1000>; /* MIPI_TX0 */ + clocks = <&mmsys MM_DSI0_ENGINE>, <&mmsys MM_DSI0_DIGITAL>; + clock-names = "engine", "digital"; + + port { + dsi0_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; +};