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

On Tue, Nov 9, 2021 at 11:34 AM Tim Harvey tharvey@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&archi... [2] https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=359775&am...

---

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@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@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@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@0"

•                           port@0 {
  

•                                   reg = <0>;
  

•                                   dsim_from_lcdif: endpoint {
  

•                                           remote-endpoint = <&lcdif_to_dsim>;
  

•                                   };
  

•                           };
  

•                   };
  

•                   csi: csi@32e20000 {
                            compatible = "fsl,imx8mm-csi", "fsl,imx7-csi";
                            reg = <0x32e20000 0x1000>;
  

-- 2.17.1