Re: [RFC PATCH v2 2/3] arm64: dts: imx8mm: Add MIPI DSI pipeline

[Tim Harvey <tharvey@gateworks.com>]

On Thu, Nov 11, 2021 at 2:15 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Add MIPI DSI pipeline for i.MX8MM.
>
> Video pipeline start from eLCDIF to MIPI DSI and respective
> Panel or Bridge on the backend side.
>
> Add support for it.

Jagan,

Thanks for your continued work on IMX8MM DSI support!

It doesn't look like you sent this to the Device Tree mainling list so
I added that to cc.

>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 55 +++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index caeb93313413..eddf3a467fd2 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -188,6 +188,12 @@ clk_ext4: clock-ext4 {
>                 clock-output-names = "clk_ext4";
>         };
>
> +       mipi_phy: mipi-video-phy {
> +               compatible = "fsl,imx8mm-mipi-video-phy";
> +               syscon = <&disp_blk_ctrl>;
> +               #phy-cells = <1>;
> +       };
> +
>         psci {
>                 compatible = "arm,psci-1.0";
>                 method = "smc";
> @@ -1085,6 +1091,55 @@ lcdif: lcdif@32e00000 {
>                                 interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
>                                 power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_LCDIF>;
>                                 status = "disabled";
> +
> +                               port {
> +                                       lcdif_out_dsi: endpoint {
> +                                               remote-endpoint = <&dsi_in_lcdif>;
> +                                       };
> +                               };
> +                       };
> +
> +                       dsi: dsi@32e10000 {

I wonder if this should this be 'mipi_dsi' like the CSI bindings
Adam's submitted here:
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20211106155427.753197-2-aford173@gmail.com/

> +                               compatible = "fsl,imx8mm-mipi-dsim";
> +                               reg = <0x32e10000 0x400>;
> +                               clocks = <&clk IMX8MM_CLK_DSI_CORE>,
> +                                        <&clk IMX8MM_CLK_DSI_PHY_REF>;
> +                               clock-names = "bus_clk", "sclk_mipi";
> +                               assigned-clocks = <&clk IMX8MM_CLK_DSI_CORE>,
> +                                                 <&clk IMX8MM_VIDEO_PLL1_OUT>,
> +                                                 <&clk IMX8MM_CLK_DSI_PHY_REF>;
> +                               assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_266M>,
> +                                                        <&clk IMX8MM_VIDEO_PLL1_BYPASS>,
> +                                                        <&clk IMX8MM_VIDEO_PLL1_OUT>;
> +                               assigned-clock-rates = <266000000>, <594000000>, <27000000>;
> +                               interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
> +                               phys = <&mipi_phy 0>;
> +                               phy-names = "dsim";
> +                               power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_MIPI_DSI>;
> +                               samsung,burst-clock-frequency = <891000000>;
> +                               samsung,esc-clock-frequency = <54000000>;
> +                               samsung,pll-clock-frequency = <27000000>;
> +                               status = "disabled";
> +
> +                               ports {
> +                                       #address-cells = <1>;
> +                                       #size-cells = <0>;
> +
> +                                       port@0 {
> +                                               reg = <0>;
> +                                               #address-cells = <1>;
> +                                               #size-cells = <0>;

I don't think the '#address-cells' and '#size-cells' are needed here
but I defer to the dt experts!

> +
> +                                               dsi_in_lcdif: endpoint@0 {
> +                                                       reg = <0>;

Per Adam's comment to my posting this should be just "port {" and we
can get rid of the @0 and the "reg=0"

Best regards,

Tim

> +                                                       remote-endpoint = <&lcdif_out_dsi>;
> +                                               };
> +                                       };
> +
> +                                       port@1 {
> +                                               reg = <1>;
> +                                       };
> +                               };
>                         };
>
>                         disp_blk_ctrl: blk-ctrl@32e28000 {
> --
> 2.25.1
>