The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
Signed-off-by: Guido Günther <[email protected]>
Tested-by: Robert Chiras <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
Acked-by: Sam Ravnborg <[email protected]>
Reviewed-by: Fabio Estevam <[email protected]>
---
.../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
1 file changed, 226 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
new file mode 100644
index 000000000000..8aff2d68fc33
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
@@ -0,0 +1,226 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Northwest Logic MIPI-DSI controller on i.MX SoCs
+
+maintainers:
+ - Guido Gúnther <[email protected]>
+ - Robert Chiras <[email protected]>
+
+description: |
+ NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
+ the SOCs NWL MIPI-DSI host controller.
+
+properties:
+ compatible:
+ const: fsl,imx8mq-nwl-dsi
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ clocks:
+ items:
+ - description: DSI core clock
+ - description: RX_ESC clock (used in escape mode)
+ - description: TX_ESC clock (used in escape mode)
+ - description: PHY_REF clock
+ - description: LCDIF clock
+
+ clock-names:
+ items:
+ - const: core
+ - const: rx_esc
+ - const: tx_esc
+ - const: phy_ref
+ - const: lcdif
+
+ mux-controls:
+ description:
+ mux controller node to use for operating the input mux
+
+ phys:
+ maxItems: 1
+ description:
+ A phandle to the phy module representing the DPHY
+
+ phy-names:
+ items:
+ - const: dphy
+
+ power-domains:
+ maxItems: 1
+
+ resets:
+ items:
+ - description: dsi byte reset line
+ - description: dsi dpi reset line
+ - description: dsi esc reset line
+ - description: dsi pclk reset line
+
+ reset-names:
+ items:
+ - const: byte
+ - const: dpi
+ - const: esc
+ - const: pclk
+
+ ports:
+ type: object
+ description:
+ A node containing DSI input & output port nodes with endpoint
+ definitions as documented in
+ Documentation/devicetree/bindings/graph.txt.
+ properties:
+ port@0:
+ type: object
+ description:
+ Input port node to receive pixel data from the
+ display controller. Exactly one endpoint must be
+ specified.
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ endpoint@0:
+ description: sub-node describing the input from LCDIF
+ type: object
+
+ endpoint@1:
+ description: sub-node describing the input from DCSS
+ type: object
+
+ reg:
+ const: 0
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+ - reg
+
+ oneOf:
+ - required:
+ - endpoint@0
+ - required:
+ - endpoint@1
+
+ additionalProperties: false
+
+ port@1:
+ type: object
+ description:
+ DSI output port node to the panel or the next bridge
+ in the chain
+
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+ - port@0
+ - port@1
+
+ additionalProperties: false
+
+patternProperties:
+ "^panel@[0-9]+$":
+ type: object
+
+required:
+ - '#address-cells'
+ - '#size-cells'
+ - clock-names
+ - clocks
+ - compatible
+ - interrupts
+ - mux-controls
+ - phy-names
+ - phys
+ - ports
+ - reg
+ - reset-names
+ - resets
+
+additionalProperties: false
+
+examples:
+ - |
+
+ #include <dt-bindings/clock/imx8mq-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/reset/imx8mq-reset.h>
+
+ mipi_dsi: mipi_dsi@30a00000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,imx8mq-nwl-dsi";
+ reg = <0x30A00000 0x300>;
+ clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
+ <&clk IMX8MQ_CLK_DSI_AHB>,
+ <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
+ <&clk IMX8MQ_CLK_DSI_PHY_REF>,
+ <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
+ clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
+ interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
+ mux-controls = <&mux 0>;
+ power-domains = <&pgc_mipi>;
+ resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
+ <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
+ <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
+ <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
+ reset-names = "byte", "dpi", "esc", "pclk";
+ phys = <&dphy>;
+ phy-names = "dphy";
+
+ panel@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "rocktech,jh057n00900";
+ reg = <0>;
+ port@0 {
+ reg = <0>;
+ panel_in: endpoint {
+ remote-endpoint = <&mipi_dsi_out>;
+ };
+ };
+ };
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #size-cells = <0>;
+ #address-cells = <1>;
+ reg = <0>;
+ mipi_dsi_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lcdif_mipi_dsi>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ mipi_dsi_out: endpoint {
+ remote-endpoint = <&panel_in>;
+ };
+ };
+ };
+ };
--
2.23.0
Hi Guido,
Thank you for the patch.
On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido Günther wrote:
> The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
>
> Signed-off-by: Guido Günther <[email protected]>
> Tested-by: Robert Chiras <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> Acked-by: Sam Ravnborg <[email protected]>
> Reviewed-by: Fabio Estevam <[email protected]>
> ---
> .../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
> 1 file changed, 226 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> new file mode 100644
> index 000000000000..8aff2d68fc33
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> @@ -0,0 +1,226 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> +
> +maintainers:
> + - Guido Gúnther <[email protected]>
> + - Robert Chiras <[email protected]>
> +
> +description: |
> + NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> + the SOCs NWL MIPI-DSI host controller.
> +
> +properties:
> + compatible:
> + const: fsl,imx8mq-nwl-dsi
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + clocks:
> + items:
> + - description: DSI core clock
> + - description: RX_ESC clock (used in escape mode)
> + - description: TX_ESC clock (used in escape mode)
> + - description: PHY_REF clock
> + - description: LCDIF clock
> +
> + clock-names:
> + items:
> + - const: core
> + - const: rx_esc
> + - const: tx_esc
> + - const: phy_ref
> + - const: lcdif
> +
> + mux-controls:
> + description:
> + mux controller node to use for operating the input mux
> +
> + phys:
> + maxItems: 1
> + description:
> + A phandle to the phy module representing the DPHY
> +
> + phy-names:
> + items:
> + - const: dphy
> +
> + power-domains:
> + maxItems: 1
> +
> + resets:
> + items:
> + - description: dsi byte reset line
> + - description: dsi dpi reset line
> + - description: dsi esc reset line
> + - description: dsi pclk reset line
> +
> + reset-names:
> + items:
> + - const: byte
> + - const: dpi
> + - const: esc
> + - const: pclk
> +
> + ports:
> + type: object
> + description:
> + A node containing DSI input & output port nodes with endpoint
> + definitions as documented in
> + Documentation/devicetree/bindings/graph.txt.
> + properties:
> + port@0:
> + type: object
> + description:
> + Input port node to receive pixel data from the
> + display controller. Exactly one endpoint must be
> + specified.
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + endpoint@0:
> + description: sub-node describing the input from LCDIF
> + type: object
> +
> + endpoint@1:
> + description: sub-node describing the input from DCSS
> + type: object
This models the two inputs to the IP core, that are connected to a mux
internally, controlled through mux-controls, right ? Why is a single
endpoint supported then, if there are two connections at the hardware
level, and why is this using endpoints instead of ports as there are
really two input ports ?
Apart from that the bindings look ok to me.
> +
> + reg:
> + const: 0
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> + - reg
> +
> + oneOf:
> + - required:
> + - endpoint@0
> + - required:
> + - endpoint@1
> +
> + additionalProperties: false
> +
> + port@1:
> + type: object
> + description:
> + DSI output port node to the panel or the next bridge
> + in the chain
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + required:
> + - '#address-cells'
> + - '#size-cells'
> + - port@0
> + - port@1
> +
> + additionalProperties: false
> +
> +patternProperties:
> + "^panel@[0-9]+$":
> + type: object
> +
> +required:
> + - '#address-cells'
> + - '#size-cells'
> + - clock-names
> + - clocks
> + - compatible
> + - interrupts
> + - mux-controls
> + - phy-names
> + - phys
> + - ports
> + - reg
> + - reset-names
> + - resets
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +
> + #include <dt-bindings/clock/imx8mq-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/reset/imx8mq-reset.h>
> +
> + mipi_dsi: mipi_dsi@30a00000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "fsl,imx8mq-nwl-dsi";
> + reg = <0x30A00000 0x300>;
> + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> + <&clk IMX8MQ_CLK_DSI_AHB>,
> + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> + <&clk IMX8MQ_CLK_DSI_PHY_REF>,
> + <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
> + clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
> + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> + mux-controls = <&mux 0>;
> + power-domains = <&pgc_mipi>;
> + resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
> + <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
> + <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
> + <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
> + reset-names = "byte", "dpi", "esc", "pclk";
> + phys = <&dphy>;
> + phy-names = "dphy";
> +
> + panel@0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "rocktech,jh057n00900";
> + reg = <0>;
> + port@0 {
> + reg = <0>;
> + panel_in: endpoint {
> + remote-endpoint = <&mipi_dsi_out>;
> + };
> + };
> + };
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + #size-cells = <0>;
> + #address-cells = <1>;
> + reg = <0>;
> + mipi_dsi_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&lcdif_mipi_dsi>;
> + };
> + };
> + port@1 {
> + reg = <1>;
> + mipi_dsi_out: endpoint {
> + remote-endpoint = <&panel_in>;
> + };
> + };
> + };
> + };
--
Regards,
Laurent Pinchart
Hi Laurent,
On Fri, Apr 10, 2020 at 02:23:42PM +0300, Laurent Pinchart wrote:
> Hi Guido,
>
> Thank you for the patch.
>
> On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido G?nther wrote:
> > The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> >
> > Signed-off-by: Guido G?nther <[email protected]>
> > Tested-by: Robert Chiras <[email protected]>
> > Reviewed-by: Rob Herring <[email protected]>
> > Acked-by: Sam Ravnborg <[email protected]>
> > Reviewed-by: Fabio Estevam <[email protected]>
> > ---
> > .../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
> > 1 file changed, 226 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > new file mode 100644
> > index 000000000000..8aff2d68fc33
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > @@ -0,0 +1,226 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> > +
> > +maintainers:
> > + - Guido G?nther <[email protected]>
> > + - Robert Chiras <[email protected]>
> > +
> > +description: |
> > + NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> > + the SOCs NWL MIPI-DSI host controller.
> > +
> > +properties:
> > + compatible:
> > + const: fsl,imx8mq-nwl-dsi
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + clocks:
> > + items:
> > + - description: DSI core clock
> > + - description: RX_ESC clock (used in escape mode)
> > + - description: TX_ESC clock (used in escape mode)
> > + - description: PHY_REF clock
> > + - description: LCDIF clock
> > +
> > + clock-names:
> > + items:
> > + - const: core
> > + - const: rx_esc
> > + - const: tx_esc
> > + - const: phy_ref
> > + - const: lcdif
> > +
> > + mux-controls:
> > + description:
> > + mux controller node to use for operating the input mux
> > +
> > + phys:
> > + maxItems: 1
> > + description:
> > + A phandle to the phy module representing the DPHY
> > +
> > + phy-names:
> > + items:
> > + - const: dphy
> > +
> > + power-domains:
> > + maxItems: 1
> > +
> > + resets:
> > + items:
> > + - description: dsi byte reset line
> > + - description: dsi dpi reset line
> > + - description: dsi esc reset line
> > + - description: dsi pclk reset line
> > +
> > + reset-names:
> > + items:
> > + - const: byte
> > + - const: dpi
> > + - const: esc
> > + - const: pclk
> > +
> > + ports:
> > + type: object
> > + description:
> > + A node containing DSI input & output port nodes with endpoint
> > + definitions as documented in
> > + Documentation/devicetree/bindings/graph.txt.
> > + properties:
> > + port@0:
> > + type: object
> > + description:
> > + Input port node to receive pixel data from the
> > + display controller. Exactly one endpoint must be
> > + specified.
> > + properties:
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + endpoint@0:
> > + description: sub-node describing the input from LCDIF
> > + type: object
> > +
> > + endpoint@1:
> > + description: sub-node describing the input from DCSS
> > + type: object
>
> This models the two inputs to the IP core, that are connected to a mux
> internally, controlled through mux-controls, right ? Why is a single
> endpoint supported then, if there are two connections at the hardware
> level, and why is this using endpoints instead of ports as there are
> really two input ports ?
That came out of
https://lore.kernel.org/linux-arm-kernel/[email protected]/
# If the ip has separate lines for DCSS and LCDIF you should distinguish
# by port number. If they are shared
# you can use endpoint number to specify DCSS or LCDIF, in both cases
# bindings should be adjusted.
I read that as
- distinguish by endpoint number:
eLCDIF--\ |
----| nwl
DCSS----/ |
- distinguish by port number:
eLCDIF-------|
| nwl
DCSS --------|
From the imx8mq ref manual i didn't see separate input lines for DCSS vs
eLCDIF the the NWL IP so i went with endpoints instead of ports. I'm
happy to change that if i got it wrong.
Cheers,
-- Guido
>
> Apart from that the bindings look ok to me.
>
> > +
> > + reg:
> > + const: 0
> > +
> > + required:
> > + - '#address-cells'
> > + - '#size-cells'
> > + - reg
> > +
> > + oneOf:
> > + - required:
> > + - endpoint@0
> > + - required:
> > + - endpoint@1
> > +
> > + additionalProperties: false
> > +
> > + port@1:
> > + type: object
> > + description:
> > + DSI output port node to the panel or the next bridge
> > + in the chain
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + required:
> > + - '#address-cells'
> > + - '#size-cells'
> > + - port@0
> > + - port@1
> > +
> > + additionalProperties: false
> > +
> > +patternProperties:
> > + "^panel@[0-9]+$":
> > + type: object
> > +
> > +required:
> > + - '#address-cells'
> > + - '#size-cells'
> > + - clock-names
> > + - clocks
> > + - compatible
> > + - interrupts
> > + - mux-controls
> > + - phy-names
> > + - phys
> > + - ports
> > + - reg
> > + - reset-names
> > + - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +
> > + #include <dt-bindings/clock/imx8mq-clock.h>
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/reset/imx8mq-reset.h>
> > +
> > + mipi_dsi: mipi_dsi@30a00000 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + compatible = "fsl,imx8mq-nwl-dsi";
> > + reg = <0x30A00000 0x300>;
> > + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> > + <&clk IMX8MQ_CLK_DSI_AHB>,
> > + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> > + <&clk IMX8MQ_CLK_DSI_PHY_REF>,
> > + <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
> > + clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
> > + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> > + mux-controls = <&mux 0>;
> > + power-domains = <&pgc_mipi>;
> > + resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
> > + <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
> > + <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
> > + <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
> > + reset-names = "byte", "dpi", "esc", "pclk";
> > + phys = <&dphy>;
> > + phy-names = "dphy";
> > +
> > + panel@0 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + compatible = "rocktech,jh057n00900";
> > + reg = <0>;
> > + port@0 {
> > + reg = <0>;
> > + panel_in: endpoint {
> > + remote-endpoint = <&mipi_dsi_out>;
> > + };
> > + };
> > + };
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + port@0 {
> > + #size-cells = <0>;
> > + #address-cells = <1>;
> > + reg = <0>;
> > + mipi_dsi_in: endpoint@0 {
> > + reg = <0>;
> > + remote-endpoint = <&lcdif_mipi_dsi>;
> > + };
> > + };
> > + port@1 {
> > + reg = <1>;
> > + mipi_dsi_out: endpoint {
> > + remote-endpoint = <&panel_in>;
> > + };
> > + };
> > + };
> > + };
>
> --
> Regards,
>
> Laurent Pinchart
>
Hi Guido,
On Fri, Apr 10, 2020 at 02:45:16PM +0200, Guido Günther wrote:
> On Fri, Apr 10, 2020 at 02:23:42PM +0300, Laurent Pinchart wrote:
> > On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido Günther wrote:
> > > The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> > >
> > > Signed-off-by: Guido Günther <[email protected]>
> > > Tested-by: Robert Chiras <[email protected]>
> > > Reviewed-by: Rob Herring <[email protected]>
> > > Acked-by: Sam Ravnborg <[email protected]>
> > > Reviewed-by: Fabio Estevam <[email protected]>
> > > ---
> > > .../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
> > > 1 file changed, 226 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > new file mode 100644
> > > index 000000000000..8aff2d68fc33
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > @@ -0,0 +1,226 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> > > +
> > > +maintainers:
> > > + - Guido Gúnther <[email protected]>
> > > + - Robert Chiras <[email protected]>
> > > +
> > > +description: |
> > > + NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> > > + the SOCs NWL MIPI-DSI host controller.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: fsl,imx8mq-nwl-dsi
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + '#address-cells':
> > > + const: 1
> > > +
> > > + '#size-cells':
> > > + const: 0
> > > +
> > > + clocks:
> > > + items:
> > > + - description: DSI core clock
> > > + - description: RX_ESC clock (used in escape mode)
> > > + - description: TX_ESC clock (used in escape mode)
> > > + - description: PHY_REF clock
> > > + - description: LCDIF clock
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: core
> > > + - const: rx_esc
> > > + - const: tx_esc
> > > + - const: phy_ref
> > > + - const: lcdif
> > > +
> > > + mux-controls:
> > > + description:
> > > + mux controller node to use for operating the input mux
> > > +
> > > + phys:
> > > + maxItems: 1
> > > + description:
> > > + A phandle to the phy module representing the DPHY
> > > +
> > > + phy-names:
> > > + items:
> > > + - const: dphy
> > > +
> > > + power-domains:
> > > + maxItems: 1
> > > +
> > > + resets:
> > > + items:
> > > + - description: dsi byte reset line
> > > + - description: dsi dpi reset line
> > > + - description: dsi esc reset line
> > > + - description: dsi pclk reset line
> > > +
> > > + reset-names:
> > > + items:
> > > + - const: byte
> > > + - const: dpi
> > > + - const: esc
> > > + - const: pclk
> > > +
> > > + ports:
> > > + type: object
> > > + description:
> > > + A node containing DSI input & output port nodes with endpoint
> > > + definitions as documented in
> > > + Documentation/devicetree/bindings/graph.txt.
> > > + properties:
> > > + port@0:
> > > + type: object
> > > + description:
> > > + Input port node to receive pixel data from the
> > > + display controller. Exactly one endpoint must be
> > > + specified.
> > > + properties:
> > > + '#address-cells':
> > > + const: 1
> > > +
> > > + '#size-cells':
> > > + const: 0
> > > +
> > > + endpoint@0:
> > > + description: sub-node describing the input from LCDIF
> > > + type: object
> > > +
> > > + endpoint@1:
> > > + description: sub-node describing the input from DCSS
> > > + type: object
> >
> > This models the two inputs to the IP core, that are connected to a mux
> > internally, controlled through mux-controls, right ? Why is a single
> > endpoint supported then, if there are two connections at the hardware
> > level, and why is this using endpoints instead of ports as there are
> > really two input ports ?
>
> That came out of
>
> https://lore.kernel.org/linux-arm-kernel/[email protected]/
>
> # If the ip has separate lines for DCSS and LCDIF you should distinguish
> # by port number. If they are shared
> # you can use endpoint number to specify DCSS or LCDIF, in both cases
> # bindings should be adjusted.
>
> I read that as
>
> - distinguish by endpoint number:
>
> eLCDIF--\ |
> ----| nwl
> DCSS----/ |
>
> - distinguish by port number:
>
> eLCDIF-------|
> | nwl
> DCSS --------|
I fully agree with you here, but in the first case I would expect the
mux to be outside of the NWL, while in the second case it would be
inside. If I understand the issue correctly, the mux is not part of the
NWL, right ? The endpoint model would then be good, but leaves the issue
of where to control the mux. There's a DT binding for video muxes in
Documentation/devicetree/bindings/media/video-mux.txt, but that would
require some support on the driver side. Do you think it would be a good
way forward ?
The alternative is to consider the mux + NWL as one device (more or less
an i.MX8M-specific integration wrapper of the NWL), but in that case
there should be two ports I believe.
> From the imx8mq ref manual i didn't see separate input lines for DCSS vs
> eLCDIF the the NWL IP so i went with endpoints instead of ports. I'm
> happy to change that if i got it wrong.
>
> > Apart from that the bindings look ok to me.
> >
> > > +
> > > + reg:
> > > + const: 0
> > > +
> > > + required:
> > > + - '#address-cells'
> > > + - '#size-cells'
> > > + - reg
> > > +
> > > + oneOf:
> > > + - required:
> > > + - endpoint@0
> > > + - required:
> > > + - endpoint@1
> > > +
> > > + additionalProperties: false
> > > +
> > > + port@1:
> > > + type: object
> > > + description:
> > > + DSI output port node to the panel or the next bridge
> > > + in the chain
> > > +
> > > + '#address-cells':
> > > + const: 1
> > > +
> > > + '#size-cells':
> > > + const: 0
> > > +
> > > + required:
> > > + - '#address-cells'
> > > + - '#size-cells'
> > > + - port@0
> > > + - port@1
> > > +
> > > + additionalProperties: false
> > > +
> > > +patternProperties:
> > > + "^panel@[0-9]+$":
> > > + type: object
> > > +
> > > +required:
> > > + - '#address-cells'
> > > + - '#size-cells'
> > > + - clock-names
> > > + - clocks
> > > + - compatible
> > > + - interrupts
> > > + - mux-controls
> > > + - phy-names
> > > + - phys
> > > + - ports
> > > + - reg
> > > + - reset-names
> > > + - resets
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > +
> > > + #include <dt-bindings/clock/imx8mq-clock.h>
> > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > + #include <dt-bindings/reset/imx8mq-reset.h>
> > > +
> > > + mipi_dsi: mipi_dsi@30a00000 {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + compatible = "fsl,imx8mq-nwl-dsi";
> > > + reg = <0x30A00000 0x300>;
> > > + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> > > + <&clk IMX8MQ_CLK_DSI_AHB>,
> > > + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> > > + <&clk IMX8MQ_CLK_DSI_PHY_REF>,
> > > + <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
> > > + clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
> > > + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> > > + mux-controls = <&mux 0>;
> > > + power-domains = <&pgc_mipi>;
> > > + resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
> > > + <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
> > > + <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
> > > + <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
> > > + reset-names = "byte", "dpi", "esc", "pclk";
> > > + phys = <&dphy>;
> > > + phy-names = "dphy";
> > > +
> > > + panel@0 {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + compatible = "rocktech,jh057n00900";
> > > + reg = <0>;
> > > + port@0 {
> > > + reg = <0>;
> > > + panel_in: endpoint {
> > > + remote-endpoint = <&mipi_dsi_out>;
> > > + };
> > > + };
> > > + };
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + port@0 {
> > > + #size-cells = <0>;
> > > + #address-cells = <1>;
> > > + reg = <0>;
> > > + mipi_dsi_in: endpoint@0 {
> > > + reg = <0>;
> > > + remote-endpoint = <&lcdif_mipi_dsi>;
> > > + };
> > > + };
> > > + port@1 {
> > > + reg = <1>;
> > > + mipi_dsi_out: endpoint {
> > > + remote-endpoint = <&panel_in>;
> > > + };
> > > + };
> > > + };
> > > + };
--
Regards,
Laurent Pinchart
Hi Laurent,
On Fri, Apr 10, 2020 at 03:57:32PM +0300, Laurent Pinchart wrote:
> Hi Guido,
>
> On Fri, Apr 10, 2020 at 02:45:16PM +0200, Guido G?nther wrote:
> > On Fri, Apr 10, 2020 at 02:23:42PM +0300, Laurent Pinchart wrote:
> > > On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido G?nther wrote:
> > > > The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> > > >
> > > > Signed-off-by: Guido G?nther <[email protected]>
> > > > Tested-by: Robert Chiras <[email protected]>
> > > > Reviewed-by: Rob Herring <[email protected]>
> > > > Acked-by: Sam Ravnborg <[email protected]>
> > > > Reviewed-by: Fabio Estevam <[email protected]>
> > > > ---
> > > > .../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
> > > > 1 file changed, 226 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > > new file mode 100644
> > > > index 000000000000..8aff2d68fc33
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> > > > @@ -0,0 +1,226 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> > > > +
> > > > +maintainers:
> > > > + - Guido G?nther <[email protected]>
> > > > + - Robert Chiras <[email protected]>
> > > > +
> > > > +description: |
> > > > + NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> > > > + the SOCs NWL MIPI-DSI host controller.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: fsl,imx8mq-nwl-dsi
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + interrupts:
> > > > + maxItems: 1
> > > > +
> > > > + '#address-cells':
> > > > + const: 1
> > > > +
> > > > + '#size-cells':
> > > > + const: 0
> > > > +
> > > > + clocks:
> > > > + items:
> > > > + - description: DSI core clock
> > > > + - description: RX_ESC clock (used in escape mode)
> > > > + - description: TX_ESC clock (used in escape mode)
> > > > + - description: PHY_REF clock
> > > > + - description: LCDIF clock
> > > > +
> > > > + clock-names:
> > > > + items:
> > > > + - const: core
> > > > + - const: rx_esc
> > > > + - const: tx_esc
> > > > + - const: phy_ref
> > > > + - const: lcdif
> > > > +
> > > > + mux-controls:
> > > > + description:
> > > > + mux controller node to use for operating the input mux
> > > > +
> > > > + phys:
> > > > + maxItems: 1
> > > > + description:
> > > > + A phandle to the phy module representing the DPHY
> > > > +
> > > > + phy-names:
> > > > + items:
> > > > + - const: dphy
> > > > +
> > > > + power-domains:
> > > > + maxItems: 1
> > > > +
> > > > + resets:
> > > > + items:
> > > > + - description: dsi byte reset line
> > > > + - description: dsi dpi reset line
> > > > + - description: dsi esc reset line
> > > > + - description: dsi pclk reset line
> > > > +
> > > > + reset-names:
> > > > + items:
> > > > + - const: byte
> > > > + - const: dpi
> > > > + - const: esc
> > > > + - const: pclk
> > > > +
> > > > + ports:
> > > > + type: object
> > > > + description:
> > > > + A node containing DSI input & output port nodes with endpoint
> > > > + definitions as documented in
> > > > + Documentation/devicetree/bindings/graph.txt.
> > > > + properties:
> > > > + port@0:
> > > > + type: object
> > > > + description:
> > > > + Input port node to receive pixel data from the
> > > > + display controller. Exactly one endpoint must be
> > > > + specified.
> > > > + properties:
> > > > + '#address-cells':
> > > > + const: 1
> > > > +
> > > > + '#size-cells':
> > > > + const: 0
> > > > +
> > > > + endpoint@0:
> > > > + description: sub-node describing the input from LCDIF
> > > > + type: object
> > > > +
> > > > + endpoint@1:
> > > > + description: sub-node describing the input from DCSS
> > > > + type: object
> > >
> > > This models the two inputs to the IP core, that are connected to a mux
> > > internally, controlled through mux-controls, right ? Why is a single
> > > endpoint supported then, if there are two connections at the hardware
> > > level, and why is this using endpoints instead of ports as there are
> > > really two input ports ?
> >
> > That came out of
> >
> > https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >
> > # If the ip has separate lines for DCSS and LCDIF you should distinguish
> > # by port number. If they are shared
> > # you can use endpoint number to specify DCSS or LCDIF, in both cases
> > # bindings should be adjusted.
> >
> > I read that as
> >
> > - distinguish by endpoint number:
> >
> > eLCDIF--\ |
> > ----| nwl
> > DCSS----/ |
> >
> > - distinguish by port number:
> >
> > eLCDIF-------|
> > | nwl
> > DCSS --------|
>
> I fully agree with you here, but in the first case I would expect the
> mux to be outside of the NWL, while in the second case it would be
> inside. If I understand the issue correctly, the mux is not part of the
> NWL, right ? The endpoint model would then be good, but leaves the issue
> Documentation/devicetree/bindings/media/video-mux.txt, but that would
> require some support on the driver side. Do you think it would be a good
> way forward ?
Binding wise that looks like a good fit. I then thought about the
implementation and figured we'd end up with something like a bridge that
basically muxes inputs - or did you have something different in mind? That
sounds generally useful when we e.g. look at runtime switching the input
display controller.
> The alternative is to consider the mux + NWL as one device (more or less
> an i.MX8M-specific integration wrapper of the NWL), but in that case
> there should be two ports I believe.
I'd go with that for the moment and fold in the above at a later
point if needed. I'm happy to work on that but would like to work on
some parts of the imx8mq display stack first.
Cheers,
-- Guido
> > From the imx8mq ref manual i didn't see separate input lines for DCSS vs
> > eLCDIF the the NWL IP so i went with endpoints instead of ports. I'm
> > happy to change that if i got it wrong.
> >
> > > Apart from that the bindings look ok to me.
> > >
> > > > +
> > > > + reg:
> > > > + const: 0
> > > > +
> > > > + required:
> > > > + - '#address-cells'
> > > > + - '#size-cells'
> > > > + - reg
> > > > +
> > > > + oneOf:
> > > > + - required:
> > > > + - endpoint@0
> > > > + - required:
> > > > + - endpoint@1
> > > > +
> > > > + additionalProperties: false
> > > > +
> > > > + port@1:
> > > > + type: object
> > > > + description:
> > > > + DSI output port node to the panel or the next bridge
> > > > + in the chain
> > > > +
> > > > + '#address-cells':
> > > > + const: 1
> > > > +
> > > > + '#size-cells':
> > > > + const: 0
> > > > +
> > > > + required:
> > > > + - '#address-cells'
> > > > + - '#size-cells'
> > > > + - port@0
> > > > + - port@1
> > > > +
> > > > + additionalProperties: false
> > > > +
> > > > +patternProperties:
> > > > + "^panel@[0-9]+$":
> > > > + type: object
> > > > +
> > > > +required:
> > > > + - '#address-cells'
> > > > + - '#size-cells'
> > > > + - clock-names
> > > > + - clocks
> > > > + - compatible
> > > > + - interrupts
> > > > + - mux-controls
> > > > + - phy-names
> > > > + - phys
> > > > + - ports
> > > > + - reg
> > > > + - reset-names
> > > > + - resets
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > + - |
> > > > +
> > > > + #include <dt-bindings/clock/imx8mq-clock.h>
> > > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > + #include <dt-bindings/reset/imx8mq-reset.h>
> > > > +
> > > > + mipi_dsi: mipi_dsi@30a00000 {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + compatible = "fsl,imx8mq-nwl-dsi";
> > > > + reg = <0x30A00000 0x300>;
> > > > + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> > > > + <&clk IMX8MQ_CLK_DSI_AHB>,
> > > > + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> > > > + <&clk IMX8MQ_CLK_DSI_PHY_REF>,
> > > > + <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
> > > > + clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
> > > > + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> > > > + mux-controls = <&mux 0>;
> > > > + power-domains = <&pgc_mipi>;
> > > > + resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
> > > > + <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
> > > > + <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
> > > > + <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
> > > > + reset-names = "byte", "dpi", "esc", "pclk";
> > > > + phys = <&dphy>;
> > > > + phy-names = "dphy";
> > > > +
> > > > + panel@0 {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + compatible = "rocktech,jh057n00900";
> > > > + reg = <0>;
> > > > + port@0 {
> > > > + reg = <0>;
> > > > + panel_in: endpoint {
> > > > + remote-endpoint = <&mipi_dsi_out>;
> > > > + };
> > > > + };
> > > > + };
> > > > +
> > > > + ports {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > +
> > > > + port@0 {
> > > > + #size-cells = <0>;
> > > > + #address-cells = <1>;
> > > > + reg = <0>;
> > > > + mipi_dsi_in: endpoint@0 {
> > > > + reg = <0>;
> > > > + remote-endpoint = <&lcdif_mipi_dsi>;
> > > > + };
> > > > + };
> > > > + port@1 {
> > > > + reg = <1>;
> > > > + mipi_dsi_out: endpoint {
> > > > + remote-endpoint = <&panel_in>;
> > > > + };
> > > > + };
> > > > + };
> > > > + };
>
> --
> Regards,
>
> Laurent Pinchart
>
Hi Guido,
On Sun, Apr 12, 2020 at 06:38:35PM +0200, Guido Günther wrote:
> On Fri, Apr 10, 2020 at 03:57:32PM +0300, Laurent Pinchart wrote:
> > On Fri, Apr 10, 2020 at 02:45:16PM +0200, Guido Günther wrote:
> >> On Fri, Apr 10, 2020 at 02:23:42PM +0300, Laurent Pinchart wrote:
> >>> On Thu, Apr 09, 2020 at 12:42:01PM +0200, Guido Günther wrote:
> >>>> The Northwest Logic MIPI DSI IP core can be found in NXPs i.MX8 SoCs.
> >>>>
> >>>> Signed-off-by: Guido Günther <[email protected]>
> >>>> Tested-by: Robert Chiras <[email protected]>
> >>>> Reviewed-by: Rob Herring <[email protected]>
> >>>> Acked-by: Sam Ravnborg <[email protected]>
> >>>> Reviewed-by: Fabio Estevam <[email protected]>
> >>>> ---
> >>>> .../bindings/display/bridge/nwl-dsi.yaml | 226 ++++++++++++++++++
> >>>> 1 file changed, 226 insertions(+)
> >>>> create mode 100644 Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..8aff2d68fc33
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/bridge/nwl-dsi.yaml
> >>>> @@ -0,0 +1,226 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>> +%YAML 1.2
> >>>> +---
> >>>> +$id: http://devicetree.org/schemas/display/bridge/nwl-dsi.yaml#
> >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>> +
> >>>> +title: Northwest Logic MIPI-DSI controller on i.MX SoCs
> >>>> +
> >>>> +maintainers:
> >>>> + - Guido Gúnther <[email protected]>
> >>>> + - Robert Chiras <[email protected]>
> >>>> +
> >>>> +description: |
> >>>> + NWL MIPI-DSI host controller found on i.MX8 platforms. This is a dsi bridge for
> >>>> + the SOCs NWL MIPI-DSI host controller.
> >>>> +
> >>>> +properties:
> >>>> + compatible:
> >>>> + const: fsl,imx8mq-nwl-dsi
> >>>> +
> >>>> + reg:
> >>>> + maxItems: 1
> >>>> +
> >>>> + interrupts:
> >>>> + maxItems: 1
> >>>> +
> >>>> + '#address-cells':
> >>>> + const: 1
> >>>> +
> >>>> + '#size-cells':
> >>>> + const: 0
> >>>> +
> >>>> + clocks:
> >>>> + items:
> >>>> + - description: DSI core clock
> >>>> + - description: RX_ESC clock (used in escape mode)
> >>>> + - description: TX_ESC clock (used in escape mode)
> >>>> + - description: PHY_REF clock
> >>>> + - description: LCDIF clock
> >>>> +
> >>>> + clock-names:
> >>>> + items:
> >>>> + - const: core
> >>>> + - const: rx_esc
> >>>> + - const: tx_esc
> >>>> + - const: phy_ref
> >>>> + - const: lcdif
> >>>> +
> >>>> + mux-controls:
> >>>> + description:
> >>>> + mux controller node to use for operating the input mux
> >>>> +
> >>>> + phys:
> >>>> + maxItems: 1
> >>>> + description:
> >>>> + A phandle to the phy module representing the DPHY
> >>>> +
> >>>> + phy-names:
> >>>> + items:
> >>>> + - const: dphy
> >>>> +
> >>>> + power-domains:
> >>>> + maxItems: 1
> >>>> +
> >>>> + resets:
> >>>> + items:
> >>>> + - description: dsi byte reset line
> >>>> + - description: dsi dpi reset line
> >>>> + - description: dsi esc reset line
> >>>> + - description: dsi pclk reset line
> >>>> +
> >>>> + reset-names:
> >>>> + items:
> >>>> + - const: byte
> >>>> + - const: dpi
> >>>> + - const: esc
> >>>> + - const: pclk
> >>>> +
> >>>> + ports:
> >>>> + type: object
> >>>> + description:
> >>>> + A node containing DSI input & output port nodes with endpoint
> >>>> + definitions as documented in
> >>>> + Documentation/devicetree/bindings/graph.txt.
> >>>> + properties:
> >>>> + port@0:
> >>>> + type: object
> >>>> + description:
> >>>> + Input port node to receive pixel data from the
> >>>> + display controller. Exactly one endpoint must be
> >>>> + specified.
> >>>> + properties:
> >>>> + '#address-cells':
> >>>> + const: 1
> >>>> +
> >>>> + '#size-cells':
> >>>> + const: 0
> >>>> +
> >>>> + endpoint@0:
> >>>> + description: sub-node describing the input from LCDIF
> >>>> + type: object
> >>>> +
> >>>> + endpoint@1:
> >>>> + description: sub-node describing the input from DCSS
> >>>> + type: object
> >>>
> >>> This models the two inputs to the IP core, that are connected to a mux
> >>> internally, controlled through mux-controls, right ? Why is a single
> >>> endpoint supported then, if there are two connections at the hardware
> >>> level, and why is this using endpoints instead of ports as there are
> >>> really two input ports ?
> >>
> >> That came out of
> >>
> >> https://lore.kernel.org/linux-arm-kernel/[email protected]/
> >>
> >> # If the ip has separate lines for DCSS and LCDIF you should distinguish
> >> # by port number. If they are shared
> >> # you can use endpoint number to specify DCSS or LCDIF, in both cases
> >> # bindings should be adjusted.
> >>
> >> I read that as
> >>
> >> - distinguish by endpoint number:
> >>
> >> eLCDIF--\ |
> >> ----| nwl
> >> DCSS----/ |
> >>
> >> - distinguish by port number:
> >>
> >> eLCDIF-------|
> >> | nwl
> >> DCSS --------|
> >
> > I fully agree with you here, but in the first case I would expect the
> > mux to be outside of the NWL, while in the second case it would be
> > inside. If I understand the issue correctly, the mux is not part of the
> > NWL, right ? The endpoint model would then be good, but leaves the issue
> > Documentation/devicetree/bindings/media/video-mux.txt, but that would
> > require some support on the driver side. Do you think it would be a good
> > way forward ?
>
> Binding wise that looks like a good fit. I then thought about the
> implementation and figured we'd end up with something like a bridge that
> basically muxes inputs - or did you have something different in mind? That
> sounds generally useful when we e.g. look at runtime switching the input
> display controller.
That's exactly what I have in mind. There's a video-mux driver in V4L2
that does exactly this, we would need something similar for DRM/KMS.
There's an additional challenge in that bridges are supposed to have a
single input and a single output at the moment, so API extensions would
be needed, which could take some time to get right. Shortcuts may be
possible to achieve a first implementation that would hardcode a
particular input, as long as the DT bindings are fine.
> > The alternative is to consider the mux + NWL as one device (more or less
> > an i.MX8M-specific integration wrapper of the NWL), but in that case
> > there should be two ports I believe.
>
> I'd go with that for the moment and fold in the above at a later
> point if needed. I'm happy to work on that but would like to work on
> some parts of the imx8mq display stack first.
There's no specific urgency on my side, if not for the fact that DT
bindings are supposed to be a stable ABI. If we have to change this, it
would be best to do it before v5.7-rc6 in order to get the changes in
v5.8. A delay until v5.8-rcX may still be acceptable as this could be
considered as a fix, but the more we wait, the bigger the risk is.
> >> From the imx8mq ref manual i didn't see separate input lines for DCSS vs
> >> eLCDIF the the NWL IP so i went with endpoints instead of ports. I'm
> >> happy to change that if i got it wrong.
> >>
> >>> Apart from that the bindings look ok to me.
> >>>
> >>>> +
> >>>> + reg:
> >>>> + const: 0
> >>>> +
> >>>> + required:
> >>>> + - '#address-cells'
> >>>> + - '#size-cells'
> >>>> + - reg
> >>>> +
> >>>> + oneOf:
> >>>> + - required:
> >>>> + - endpoint@0
> >>>> + - required:
> >>>> + - endpoint@1
> >>>> +
> >>>> + additionalProperties: false
> >>>> +
> >>>> + port@1:
> >>>> + type: object
> >>>> + description:
> >>>> + DSI output port node to the panel or the next bridge
> >>>> + in the chain
> >>>> +
> >>>> + '#address-cells':
> >>>> + const: 1
> >>>> +
> >>>> + '#size-cells':
> >>>> + const: 0
> >>>> +
> >>>> + required:
> >>>> + - '#address-cells'
> >>>> + - '#size-cells'
> >>>> + - port@0
> >>>> + - port@1
> >>>> +
> >>>> + additionalProperties: false
> >>>> +
> >>>> +patternProperties:
> >>>> + "^panel@[0-9]+$":
> >>>> + type: object
> >>>> +
> >>>> +required:
> >>>> + - '#address-cells'
> >>>> + - '#size-cells'
> >>>> + - clock-names
> >>>> + - clocks
> >>>> + - compatible
> >>>> + - interrupts
> >>>> + - mux-controls
> >>>> + - phy-names
> >>>> + - phys
> >>>> + - ports
> >>>> + - reg
> >>>> + - reset-names
> >>>> + - resets
> >>>> +
> >>>> +additionalProperties: false
> >>>> +
> >>>> +examples:
> >>>> + - |
> >>>> +
> >>>> + #include <dt-bindings/clock/imx8mq-clock.h>
> >>>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>>> + #include <dt-bindings/reset/imx8mq-reset.h>
> >>>> +
> >>>> + mipi_dsi: mipi_dsi@30a00000 {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> + compatible = "fsl,imx8mq-nwl-dsi";
> >>>> + reg = <0x30A00000 0x300>;
> >>>> + clocks = <&clk IMX8MQ_CLK_DSI_CORE>,
> >>>> + <&clk IMX8MQ_CLK_DSI_AHB>,
> >>>> + <&clk IMX8MQ_CLK_DSI_IPG_DIV>,
> >>>> + <&clk IMX8MQ_CLK_DSI_PHY_REF>,
> >>>> + <&clk IMX8MQ_CLK_LCDIF_PIXEL>;
> >>>> + clock-names = "core", "rx_esc", "tx_esc", "phy_ref", "lcdif";
> >>>> + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
> >>>> + mux-controls = <&mux 0>;
> >>>> + power-domains = <&pgc_mipi>;
> >>>> + resets = <&src IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N>,
> >>>> + <&src IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N>,
> >>>> + <&src IMX8MQ_RESET_MIPI_DSI_ESC_RESET_N>,
> >>>> + <&src IMX8MQ_RESET_MIPI_DSI_PCLK_RESET_N>;
> >>>> + reset-names = "byte", "dpi", "esc", "pclk";
> >>>> + phys = <&dphy>;
> >>>> + phy-names = "dphy";
> >>>> +
> >>>> + panel@0 {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> + compatible = "rocktech,jh057n00900";
> >>>> + reg = <0>;
> >>>> + port@0 {
> >>>> + reg = <0>;
> >>>> + panel_in: endpoint {
> >>>> + remote-endpoint = <&mipi_dsi_out>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> +
> >>>> + ports {
> >>>> + #address-cells = <1>;
> >>>> + #size-cells = <0>;
> >>>> +
> >>>> + port@0 {
> >>>> + #size-cells = <0>;
> >>>> + #address-cells = <1>;
> >>>> + reg = <0>;
> >>>> + mipi_dsi_in: endpoint@0 {
> >>>> + reg = <0>;
> >>>> + remote-endpoint = <&lcdif_mipi_dsi>;
> >>>> + };
> >>>> + };
> >>>> + port@1 {
> >>>> + reg = <1>;
> >>>> + mipi_dsi_out: endpoint {
> >>>> + remote-endpoint = <&panel_in>;
> >>>> + };
> >>>> + };
> >>>> + };
> >>>> + };
--
Regards,
Laurent Pinchart