[RFC] arm64: dts: imx8mm: Add MIPI and LCDIF nodes

[Adam Ford]

On Tue, Nov 9, 2021 at 11:34 AM Tim Harvey <tharvey at gateworks.com> wrote:
>
> Add nodes for MIPI DSI and LCDIF on IMX8MM
>
> I'm currently working with a set of patches to convert drm/exynos
> to a bridge [1] and add IMX8MM support [2] in order to get IMX8MM DSI
> working for display with a Raspberry Pi DSI touchscreen compatible with
> a Toshiba TC358762 DSI bridge and Powertip PH800480T013-IDF02
> touchscreen.
>
> I had this working on a 5.10 kernel with the old blk-ctl and
> power-domain drivers that didn't make it into mainline but my 5.15
> series with blk-ctl backported from next hangs right after
> "[drm] Initialized mxsfb-drm 1.0.0 20160824 for 32e00000.lcdif on minor 0"
> so I assume I have a power-domain not getting enabled.
>
> Please let me know if you see an issue with the way I've configured
> power-domain or clocks here.
>
> Best Regards,
>
> Tim
> [1] https://patchwork.kernel.org/project/dri-devel/list/?series=347439&archive=both&state=*
> [2] https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=359775&archive=both&state=*
> ---
>  arch/arm64/boot/dts/freescale/imx8mm.dtsi | 68 +++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index 208a0ed840f4..195dcbff7058 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -188,6 +188,12 @@
>                 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";
> @@ -1068,6 +1074,68 @@
>                         #size-cells = <1>;
>                         ranges = <0x32c00000 0x32c00000 0x400000>;
>
> +                       lcdif: lcdif at 32e00000 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;

If there is only 1 port I don't think you need address-cells and
size-cells.  See more below....

> +                               compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif";
> +                               reg = <0x32e00000 0x10000>;
> +                               clocks = <&clk IMX8MM_CLK_LCDIF_PIXEL>,
> +                                        <&clk IMX8MM_CLK_DISP_AXI_ROOT>,
> +                                        <&clk IMX8MM_CLK_DISP_APB_ROOT>;
> +                               clock-names = "pix", "disp_axi", "axi";
> +                               assigned-clocks = <&clk IMX8MM_CLK_LCDIF_PIXEL>,
> +                                                 <&clk IMX8MM_CLK_DISP_AXI>,
> +                                                 <&clk IMX8MM_CLK_DISP_APB>;
> +                               assigned-clock-parents = <&clk IMX8MM_VIDEO_PLL1_OUT>,
> +                                                        <&clk IMX8MM_SYS_PLL2_1000M>,
> +                                                        <&clk IMX8MM_SYS_PLL1_800M>;
> +                               assigned-clock-rate = <594000000>, <500000000>, <200000000>;
> +                               interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
> +                               power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_LCDIF>;
> +                               status = "disabled";
> +
> +                               port at 0 {
> +                                       reg = <0>;

This should be just "port {" and we can get rid of the @0 and the "reg=0"

This eliminates some build warnings.
> +
> +                                       lcdif_to_dsim: endpoint {
> +                                               remote-endpoint = <&dsim_from_lcdif>;
> +                                       };
> +                               };
> +                       };
> +
> +                       mipi_dsi: mipi_dsi at 32e10000 {
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;

I think this should move into a sub node...see below

> +                               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";

I think the mipi_phy is the equivalent to the mipi_rst_mask in the
power domain I added to the CSI.  Looking through the NXP 5.10 kernel,
 BIT(17) is the reset for the MIPI_DSI.  I haven't tried it yet, but
it makes me assume that BIT(16) is for the MIPI_CSI.

Both bits are set in the imx-atf code, and the documentation isn't
clear, but I tweaked the blk-ctrl driver to only set BIT(16) for the
mipi_rst_mask on the CSI.  I still get some hanging, so it's not any
better that way.

> +                               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";
> +

Add a subnode called "ports" and move address-cells and size-cells
into it along wtih "port at 0"

> +                               port at 0 {
> +                                       reg = <0>;
> +
> +                                       dsim_from_lcdif: endpoint {
> +                                               remote-endpoint = <&lcdif_to_dsim>;
> +                                       };
> +                               };
> +                       };
> +
>                         csi: csi at 32e20000 {
>                                 compatible = "fsl,imx8mm-csi", "fsl,imx7-csi";
>                                 reg = <0x32e20000 0x1000>;
> --
> 2.17.1
>