| Message ID | 20211109173357.780-1-tharvey@gateworks.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] arm64: dts: imx8mm: Add MIPI and LCDIF nodes | expand |
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&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@32e00000 { > + #address-cells = <1>; > + #size-cells = <0>; > + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like MXSFB_V4, but with overlays and those overlays have more registers configured in the mxsfb_kms driver. Have you tried using imx28-lcdif to see if it makes a difference? > + 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>; Just through visual inspection, it appears that the IMX8MM_CLK_DISP_AXI and IMX8MM_CLK_DISP_APB clock-parents and rates are already set in the pgc_dispmix, so I think it's safe to reduce those lines to just assigning IMX8MM_CLK_LCDIF_PIXEL to the IMX8MM_VIDEO_PLL1_OUT with the assigned-clock-rate set to 594000000. adam > + interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>; > + power-domains = <&disp_blk_ctrl IMX8MM_DISPBLK_PD_LCDIF>; > + status = "disabled"; > + > + port@0 { > + reg = <0>; > + > + lcdif_to_dsim: endpoint { > + remote-endpoint = <&dsim_from_lcdif>; > + }; > + }; > + }; > + > + mipi_dsi: mipi_dsi@32e10000 { > + #address-cells = <1>; > + #size-cells = <0>; > + 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"; > + > + 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 >
On 11/9/21 8:35 PM, Adam Ford wrote: [...] >> 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>; >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > MXSFB_V4, but with overlays and those overlays have more registers > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > to see if it makes a difference? Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) ... just to make sure there is no confusion. [...] >> + mipi_dsi: mipi_dsi@32e10000 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + 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>; This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and samsung,burst-clock-frequency is really the DSI link clock which is panel/bridge specific ... but, why do we need to specify such policy in DT rather than have the panel/bridge drivers negotiate the best clock settings with DSIM bridge driver ? This should be something which should be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc samsung,*-clock-frequency properties shouldn't even be needed then. Also, are the DSIM bindings stable now ?
On Tue, Nov 9, 2021 at 11:36 AM Adam Ford <aford173@gmail.com> wrote: > > 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&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@32e00000 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > MXSFB_V4, but with overlays and those overlays have more registers > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > to see if it makes a difference? Adam, Yes, I see now a discussion regarding this [1]. Changing to 'imx28-lcdif' still hangs. [1] https://patchwork.kernel.org/project/dri-devel/patch/20210620224834.189411-1-marex@denx.de/ > > > + 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>; > > Just through visual inspection, it appears that the > IMX8MM_CLK_DISP_AXI and IMX8MM_CLK_DISP_APB clock-parents and rates > are already set in the pgc_dispmix, so I think it's safe to reduce > those lines to just assigning IMX8MM_CLK_LCDIF_PIXEL to the > IMX8MM_VIDEO_PLL1_OUT with the assigned-clock-rate set to 594000000. > Ok, that makes sense. Incidentally I added some debug prints in imx_pgc_power_{up,down} and find we hang at: imx_pgc_power_down mipi Best Regards, Tim
On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > On 11/9/21 8:35 PM, Adam Ford wrote: > > [...] > > >> 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>; > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > MXSFB_V4, but with overlays and those overlays have more registers > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > to see if it makes a difference? > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > ... just to make sure there is no confusion. > > [...] > > >> + mipi_dsi: mipi_dsi@32e10000 { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + 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>; > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > samsung,burst-clock-frequency is really the DSI link clock which is > panel/bridge specific ... but, why do we need to specify such policy in > DT rather than have the panel/bridge drivers negotiate the best clock > settings with DSIM bridge driver ? This should be something which should > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > samsung,*-clock-frequency properties shouldn't even be needed then. > > Also, are the DSIM bindings stable now ? Thanks Marek. No, there is no dsim driver yet. I'm not clear if there is still dissagreement on if the drm/exynos driver can be split up or if a whole new somewhat duplicate driver needs to be made. I know Jagan also has a series he is working on that uses drm/exynos which I believe he will share an update on in a day or so. I'm still using the work that Michael [1] and you [2] did a long time back. I had this solution working on a 5.10 kernel but it was based on the old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the issue I'm having is something not setup correctly with blk-ctl per my dt settings or perhaps something missing from the blk-ctl driver like Adam found [3] I am hanging at: [ 1.064088] imx_pgc_power_up gpumix [ 1.077506] imx_pgc_power_down gpumix [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi [ 1.102682] imx_pgc_power_up dispmix [ 1.106825] imx_pgc_power_up mipi [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not found, using dummy regulator [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, using dummy regulator [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi [ 1.143788] imx_pgc_power_down mipi [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif [ 1.147316] imx_pgc_power_down dispmix [ 1.155804] imx_pgc_power_up dispmix [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 ^^^ this is just a defer [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif [ 1.176655] imx_pgc_power_down dispmix [ 1.181790] libphy: fec_enet_mii_bus: probed [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 ^^^ not sure what this is but had it back in my 5.10 solution as well so did not investigate [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi [ 3.926936] imx_pgc_power_up dispmix [ 3.931109] imx_pgc_power_up mipi [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for 32e00000.lcdif on minor 0 ^^^ not clear why dispblk-lcdif got disabled 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=* [3] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20211106155427.753197-1-aford173@gmail.com/
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&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@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 >
On Tue, Nov 9, 2021 at 6:24 PM Adam Ford <aford173@gmail.com> wrote: > > 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. I don't get hanging, but the samsung-dsi-imx driver doesn't load automatically for some reason, so I manually modprobe it. In my configuration, I have lcdif->dsi->hdmi-bridge When I modprobe the samsung-dsim-imx driver, I get the following: [ 50.217288] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010 [ 50.217292] Mem abort info: [ 50.217294] ESR = 0x96000004 [ 50.217297] EC = 0x25: DABT (current EL), IL = 32 bits [ 50.217301] SET = 0, FnV = 0 [ 50.217303] EA = 0, S1PTW = 0 [ 50.217306] FSC = 0x04: level 0 translation fault [ 50.217309] Data abort info: [ 50.217311] ISV = 0, ISS = 0x00000004 [ 50.217313] CM = 0, WnR = 0 [ 50.217316] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000044dfb000 [ 50.217320] [0000000000000010] pgd=0000000000000000, p4d=0000000000000000 [ 50.217331] Internal error: Oops: 96000004 [#1] PREEMPT SMP [ 50.217336] Modules linked in: samsung_dsim_imx samsung_dsim 8021q garp mrp stp llc caam_jr caamhash_desc caamalg_desc snd_soc_hdmi_codec crypto_engine rng_core authenc libdes brcmfmac brcmutil cfg80211 crct10dif_ce etnaviv snd_soc_fsl_asoc_card mxsfb snd_soc_imx_audmux gpu_sched fsl_imx8_ddr_perf imx8m_ddrc spi_imx spi_bitbang adv7511 cec drm_kms_helper at24 caam clk_bd718x7 snd_soc_wm8962 rtc_pcf85363 error rtc_snvs snvs_pwrkey imx8mm_thermal snd_soc_fsl_sai imx_pcm_dma snd_soc_simple_card snd_soc_simple_card_utils imx_cpufreq_dt bluetooth display_connector ecdh_generic ecc rfkill fuse drm ipv6 [ 50.217453] CPU: 0 PID: 252 Comm: kworker/u8:4 Not tainted 5.15.0-ga6004e62a74a-dirty #3 [ 50.217459] Hardware name: Beacon EmbeddedWorks i.MX8M Mini Development Kit (DT) [ 50.217464] Workqueue: events_unbound deferred_probe_work_func [ 50.217482] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 50.217488] pc : mxsfb_crtc_atomic_enable+0x60/0x5e0 [mxsfb] [ 50.217500] lr : mxsfb_crtc_atomic_enable+0x60/0x5e0 [mxsfb] [ 50.217508] sp : ffff800012b6b500 [ 50.217510] x29: ffff800012b6b500 x28: ffff00000a52fe30 x27: 0000000000000000 [ 50.217519] x26: ffff000004fb0800 x25: 0000000000000038 x24: ffff8000091ab770 [ 50.217528] x23: 0000000000000038 x22: ffff800009150c80 x21: ffff000009f4b5e8 [ 50.217536] x20: ffff00000a538d80 x19: ffff000009f4b080 x18: fffffffffffeae28 [ 50.217544] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000048 [ 50.217552] x14: 0000000000000001 x13: 78696d7073696420 x12: fffffffffffcae27 [ 50.217560] x11: ffff800011d32ed0 x10: 000000000000000a x9 : ffff800008fc3000 [ 50.217569] x8 : ffff000003c71b80 x7 : 0000000000000000 x6 : 000000001abc391d [ 50.217577] x5 : 0000000000000130 x4 : 0000000000000000 x3 : 0000000000000000 [ 50.217585] x2 : 0000000000000000 x1 : ffff00000874b880 x0 : 0000000000000000 [ 50.217593] Call trace: [ 50.217596] mxsfb_crtc_atomic_enable+0x60/0x5e0 [mxsfb] [ 50.217604] drm_atomic_helper_commit_modeset_enables+0x208/0x250 [drm_kms_helper] [ 50.217725] drm_atomic_helper_commit_tail_rpm+0x50/0xa0 [drm_kms_helper] [ 50.217797] commit_tail+0xa4/0x184 [drm_kms_helper] [ 50.217869] drm_atomic_helper_commit+0x160/0x370 [drm_kms_helper] [ 50.217940] drm_atomic_commit+0x50/0x60 [drm] [ 50.218151] drm_client_modeset_commit_atomic+0x1c8/0x260 [drm] [ 50.218284] drm_client_modeset_commit_locked+0x60/0x19c [drm] [ 50.218413] drm_client_modeset_commit+0x34/0x60 [drm] [ 50.218543] drm_fb_helper_set_par+0xcc/0x124 [drm_kms_helper] [ 50.218649] fbcon_init+0x394/0x4e0 [ 50.218659] visual_init+0xb4/0x104 [ 50.218668] do_bind_con_driver.isra.0+0x1c4/0x394 [ 50.218676] do_take_over_console+0x144/0x1fc [ 50.218683] do_fbcon_takeover+0x6c/0xe0 [ 50.218688] fbcon_fb_registered+0x104/0x11c [ 50.218693] register_framebuffer+0x1f4/0x350 [ 50.218701] __drm_fb_helper_initial_config_and_unlock+0x338/0x534 [drm_kms_helper] [ 50.218775] drm_fbdev_client_hotplug+0x148/0x230 [drm_kms_helper] [ 50.218846] drm_fbdev_generic_setup+0xb4/0x190 [drm_kms_helper] [ 50.218919] mxsfb_probe+0x324/0x42c [mxsfb] [ 50.218929] platform_probe+0x6c/0xe0 [ 50.218938] really_probe.part.0+0x9c/0x310 [ 50.218946] __driver_probe_device+0x98/0x144 [ 50.218953] driver_probe_device+0xc8/0x15c [ 50.218959] __device_attach_driver+0xb8/0x120 [ 50.218964] bus_for_each_drv+0x78/0xd0 [ 50.218971] __device_attach+0xd8/0x180 [ 50.218977] device_initial_probe+0x18/0x24 [ 50.218982] bus_probe_device+0xa0/0xac [ 50.218988] deferred_probe_work_func+0x88/0xc0 [ 50.218995] process_one_work+0x1d0/0x354 [ 50.219001] worker_thread+0x13c/0x470 [ 50.219006] kthread+0x154/0x160 [ 50.219012] ret_from_fork+0x10/0x20 [ 50.219024] Code: f944ee61 b40000a1 aa1403e0 97f73bf5 (b9401017) [ 50.219030] ---[ end trace 8e3f6b78d85cbf0f ]--- Looking at the power domain, it appears that the LCDIf is enabled and the power domain for it is active, but the dispblk-mipi-dsi power domain is not: root@beacon-imx8mm-kit:~# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary domain status children performance /device runtime status ---------------------------------------------------------------------------------------------- vpublk-h1 off-0 0 vpublk-g2 off-0 0 vpublk-g1 off-0 0 vpumix off-0 0 /devices/genpd:0:38330000.blk-ctrl suspended 0 gpu off-0 0 /devices/platform/soc@0/38000000.gpu suspended 0 /devices/platform/soc@0/38008000.gpu suspended 0 dispblk-mipi-csi off-0 0 dispblk-mipi-dsi off-0 0 /devices/platform/soc@0/32c00000.bus/32e10000.mipi_dsi suspended 0 dispblk-lcdif on 0 /devices/platform/soc@0/32c00000.bus/32e00000.lcdif active 0 dispblk-csi-bridge off-0 0 mipi off-0 0 /devices/genpd:3:32e28000.blk-ctrl suspended 0 /devices/genpd:4:32e28000.blk-ctrl suspended 0 dispmix on 0 /devices/genpd:0:32e28000.blk-ctrl active 0 /devices/genpd:1:32e28000.blk-ctrl suspended 0 /devices/genpd:2:32e28000.blk-ctrl active 0 vpu-h1 off-0 0 /devices/genpd:3:38330000.blk-ctrl suspended 0 vpu-g2 off-0 0 /devices/genpd:2:38330000.blk-ctrl suspended 0 vpu-g1 off-0 0 /devices/genpd:1:38330000.blk-ctrl suspended 0 gpumix off-0 0 /devices/platform/soc@0/30000000.bus/303a0000.gpc/imx-pgc-domain.5 suspended 0 usb-otg2 off-0 0 usb-otg1 off-0 0 pcie off-0 0 hsiomix off-0 0 /devices/platform/soc@0/30000000.bus/303a0000.gpc/imx-pgc-domain.1 suspended 0 /devices/platform/soc@0/30000000.bus/303a0000.gpc/imx-pgc-domain.2 suspended 0 /devices/platform/soc@0/30000000.bus/303a0000.gpc/imx-pgc-domain.3 suspended 0 I'll be traveling for the rest of the week, but I'll try to help when I come back. adam > > > > 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@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 > >
On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > [...] > > > > >> 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>; > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > MXSFB_V4, but with overlays and those overlays have more registers > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > to see if it makes a difference? > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > ... just to make sure there is no confusion. > > > > [...] > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > >> + #address-cells = <1>; > > >> + #size-cells = <0>; > > >> + 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>; > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > samsung,burst-clock-frequency is really the DSI link clock which is > > panel/bridge specific ... but, why do we need to specify such policy in > > DT rather than have the panel/bridge drivers negotiate the best clock > > settings with DSIM bridge driver ? This should be something which should > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > Also, are the DSIM bindings stable now ? > > Thanks Marek. > > No, there is no dsim driver yet. I'm not clear if there is still > dissagreement on if the drm/exynos driver can be split up or if a > whole new somewhat duplicate driver needs to be made. I know Jagan > also has a series he is working on that uses drm/exynos which I > believe he will share an update on in a day or so. > > I'm still using the work that Michael [1] and you [2] did a long time back. > > I had this solution working on a 5.10 kernel but it was based on the > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > issue I'm having is something not setup correctly with blk-ctl per my > dt settings or perhaps something missing from the blk-ctl driver like > Adam found [3] > > I am hanging at: > [ 1.064088] imx_pgc_power_up gpumix > [ 1.077506] imx_pgc_power_down gpumix > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > [ 1.102682] imx_pgc_power_up dispmix > [ 1.106825] imx_pgc_power_up mipi > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > found, using dummy regulator > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > using dummy regulator > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > [ 1.143788] imx_pgc_power_down mipi > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > [ 1.147316] imx_pgc_power_down dispmix > [ 1.155804] imx_pgc_power_up dispmix > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > ^^^ this is just a defer > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > [ 1.176655] imx_pgc_power_down dispmix > [ 1.181790] libphy: fec_enet_mii_bus: probed > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > ^^^ not sure what this is but had it back in my 5.10 solution as well > so did not investigate > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > [ 3.926936] imx_pgc_power_up dispmix > [ 3.931109] imx_pgc_power_up mipi > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > 32e00000.lcdif on minor 0 > ^^^ not clear why dispblk-lcdif got disabled here I've reproduced this. look like lcdif power-domains are not notified ON. checking on the power-sequence for lcdif side. old patches from Lucas on v5.13 seems working as expected. Jagan.
On Wed, Nov 10, 2021 at 11:58 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > > > [...] > > > > > > >> 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>; > > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > > MXSFB_V4, but with overlays and those overlays have more registers > > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > > to see if it makes a difference? > > > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > > ... just to make sure there is no confusion. > > > > > > [...] > > > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > > >> + #address-cells = <1>; > > > >> + #size-cells = <0>; > > > >> + 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>; > > > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > > samsung,burst-clock-frequency is really the DSI link clock which is > > > panel/bridge specific ... but, why do we need to specify such policy in > > > DT rather than have the panel/bridge drivers negotiate the best clock > > > settings with DSIM bridge driver ? This should be something which should > > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > > > Also, are the DSIM bindings stable now ? > > > > Thanks Marek. > > > > No, there is no dsim driver yet. I'm not clear if there is still > > dissagreement on if the drm/exynos driver can be split up or if a > > whole new somewhat duplicate driver needs to be made. I know Jagan > > also has a series he is working on that uses drm/exynos which I > > believe he will share an update on in a day or so. > > > > I'm still using the work that Michael [1] and you [2] did a long time back. > > > > I had this solution working on a 5.10 kernel but it was based on the > > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > > issue I'm having is something not setup correctly with blk-ctl per my > > dt settings or perhaps something missing from the blk-ctl driver like > > Adam found [3] > > > > I am hanging at: > > [ 1.064088] imx_pgc_power_up gpumix > > [ 1.077506] imx_pgc_power_down gpumix > > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > [ 1.102682] imx_pgc_power_up dispmix > > [ 1.106825] imx_pgc_power_up mipi > > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > > found, using dummy regulator > > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > > using dummy regulator > > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > > [ 1.143788] imx_pgc_power_down mipi > > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > > [ 1.147316] imx_pgc_power_down dispmix > > [ 1.155804] imx_pgc_power_up dispmix > > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > > ^^^ this is just a defer > > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > > [ 1.176655] imx_pgc_power_down dispmix > > [ 1.181790] libphy: fec_enet_mii_bus: probed > > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > > ^^^ not sure what this is but had it back in my 5.10 solution as well > > so did not investigate > > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > [ 3.926936] imx_pgc_power_up dispmix > > [ 3.931109] imx_pgc_power_up mipi > > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > > 32e00000.lcdif on minor 0 > > ^^^ not clear why dispblk-lcdif got disabled here > > I've reproduced this. look like lcdif power-domains are not notified > ON. checking on the power-sequence for lcdif side. old patches from > Lucas on v5.13 seems working as expected. I've found the issue on lcdif atomic_enable, where the bus format is retrieved from NULL bus_state. Here is the diff for it. --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -359,13 +359,14 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, drm_crtc_vblank_on(crtc); /* If there is a bridge attached to the LCDIF, use its bus format */ - if (mxsfb->bridge) { + if (mxsfb->bridge && state) { bridge_state = drm_atomic_get_new_bridge_state(state, mxsfb->bridge); - bus_format = bridge_state->input_bus_cfg.format; + if (bridge_state) + bus_format = bridge_state->input_bus_cfg.format; I believe this can be fixed via DSIM bridge. working on for those changes, the key challenges is to make the DSIM to work even for exynos, which indeed blocker for me to validate in hardware (i don't have DSI based exynos). Meanwhile, I have posed RFC for DSIM DTS changes, please check it here. https://patchwork.kernel.org/project/dri-devel/cover/20211111101456.584061-1-jagan@amarulasolutions.com/ Jagan.
On Thu, Nov 11, 2021 at 2:19 AM Jagan Teki <jagan@amarulasolutions.com> wrote: > > On Wed, Nov 10, 2021 at 11:58 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > > > > > [...] > > > > > > > > >> 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>; > > > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > > > MXSFB_V4, but with overlays and those overlays have more registers > > > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > > > to see if it makes a difference? > > > > > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > > > ... just to make sure there is no confusion. > > > > > > > > [...] > > > > > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > > > >> + #address-cells = <1>; > > > > >> + #size-cells = <0>; > > > > >> + 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>; > > > > > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > > > samsung,burst-clock-frequency is really the DSI link clock which is > > > > panel/bridge specific ... but, why do we need to specify such policy in > > > > DT rather than have the panel/bridge drivers negotiate the best clock > > > > settings with DSIM bridge driver ? This should be something which should > > > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > > > > > Also, are the DSIM bindings stable now ? > > > > > > Thanks Marek. > > > > > > No, there is no dsim driver yet. I'm not clear if there is still > > > dissagreement on if the drm/exynos driver can be split up or if a > > > whole new somewhat duplicate driver needs to be made. I know Jagan > > > also has a series he is working on that uses drm/exynos which I > > > believe he will share an update on in a day or so. > > > > > > I'm still using the work that Michael [1] and you [2] did a long time back. > > > > > > I had this solution working on a 5.10 kernel but it was based on the > > > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > > > issue I'm having is something not setup correctly with blk-ctl per my > > > dt settings or perhaps something missing from the blk-ctl driver like > > > Adam found [3] > > > > > > I am hanging at: > > > [ 1.064088] imx_pgc_power_up gpumix > > > [ 1.077506] imx_pgc_power_down gpumix > > > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > [ 1.102682] imx_pgc_power_up dispmix > > > [ 1.106825] imx_pgc_power_up mipi > > > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > > > found, using dummy regulator > > > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > > > using dummy regulator > > > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > > > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > > > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > > > [ 1.143788] imx_pgc_power_down mipi > > > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > > > [ 1.147316] imx_pgc_power_down dispmix > > > [ 1.155804] imx_pgc_power_up dispmix > > > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > > > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > > > ^^^ this is just a defer > > > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > > > [ 1.176655] imx_pgc_power_down dispmix > > > [ 1.181790] libphy: fec_enet_mii_bus: probed > > > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > > > ^^^ not sure what this is but had it back in my 5.10 solution as well > > > so did not investigate > > > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > [ 3.926936] imx_pgc_power_up dispmix > > > [ 3.931109] imx_pgc_power_up mipi > > > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > > > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > > > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > > > 32e00000.lcdif on minor 0 > > > ^^^ not clear why dispblk-lcdif got disabled here > > > > I've reproduced this. look like lcdif power-domains are not notified > > ON. checking on the power-sequence for lcdif side. old patches from > > Lucas on v5.13 seems working as expected. > > I've found the issue on lcdif atomic_enable, where the bus format is > retrieved from NULL bus_state. Here is the diff for it. > > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > @@ -359,13 +359,14 @@ static void mxsfb_crtc_atomic_enable(struct > drm_crtc *crtc, > drm_crtc_vblank_on(crtc); > > /* If there is a bridge attached to the LCDIF, use its bus format */ > - if (mxsfb->bridge) { > + if (mxsfb->bridge && state) { > bridge_state = > drm_atomic_get_new_bridge_state(state, > mxsfb->bridge); > - bus_format = bridge_state->input_bus_cfg.format; > + if (bridge_state) > + bus_format = bridge_state->input_bus_cfg.format; >' > I believe this can be fixed via DSIM bridge. working on for those > changes, the key challenges is to make the DSIM to work even for > exynos, which indeed blocker for me to validate in hardware (i don't > have DSI based exynos). > > Meanwhile, I have posed RFC for DSIM DTS changes, please check it here. > https://patchwork.kernel.org/project/dri-devel/cover/20211111101456.584061-1-jagan@amarulasolutions.com/ > Jagan, Thank you! This does resolve the hang on my end as well. I will look at your 'arm64: imx8mm: Add MIPI DSI support' series. There was some discussion regarding giving up on trying to split up the exynos driver such that it could work with IMX8MM vs just duplicating it. I thought the recommendation was to duplicate it as it wasn't clear if there was a way to split it out without breaking current exynos DSI but I'll have to see if I can find the thread. Best regards, Tim
On Thu, 11 Nov 2021 13:21:03 -0800, Tim Harvey wrote: > On Thu, Nov 11, 2021 at 2:19 AM Jagan Teki <jagan@amarulasolutions.com> wrote: > > On Wed, Nov 10, 2021 at 11:58 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > > > > > > > [...] > > > > > > > > > > >> 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>; > > > > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > > > > MXSFB_V4, but with overlays and those overlays have more registers > > > > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > > > > to see if it makes a difference? > > > > > > > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > > > > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > > > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > > > > ... just to make sure there is no confusion. > > > > > > > > > > [...] > > > > > > > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > > > > >> + #address-cells = <1>; > > > > > >> + #size-cells = <0>; > > > > > >> + 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>; > > > > > > > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > > > > samsung,burst-clock-frequency is really the DSI link clock which is > > > > > panel/bridge specific ... but, why do we need to specify such policy in > > > > > DT rather than have the panel/bridge drivers negotiate the best clock > > > > > settings with DSIM bridge driver ? This should be something which should > > > > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > > > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > > > > > > > Also, are the DSIM bindings stable now ? > > > > > > > > Thanks Marek. > > > > > > > > No, there is no dsim driver yet. I'm not clear if there is still > > > > dissagreement on if the drm/exynos driver can be split up or if a > > > > whole new somewhat duplicate driver needs to be made. I know Jagan > > > > also has a series he is working on that uses drm/exynos which I > > > > believe he will share an update on in a day or so. > > > > > > > > I'm still using the work that Michael [1] and you [2] did a long time back. > > > > > > > > I had this solution working on a 5.10 kernel but it was based on the > > > > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > > > > issue I'm having is something not setup correctly with blk-ctl per my > > > > dt settings or perhaps something missing from the blk-ctl driver like > > > > Adam found [3] > > > > > > > > I am hanging at: > > > > [ 1.064088] imx_pgc_power_up gpumix > > > > [ 1.077506] imx_pgc_power_down gpumix > > > > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > [ 1.102682] imx_pgc_power_up dispmix > > > > [ 1.106825] imx_pgc_power_up mipi > > > > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > > > > found, using dummy regulator > > > > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > > > > using dummy regulator > > > > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > > > > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > > > > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > > > > [ 1.143788] imx_pgc_power_down mipi > > > > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > [ 1.147316] imx_pgc_power_down dispmix > > > > [ 1.155804] imx_pgc_power_up dispmix > > > > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > > > > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > > > > ^^^ this is just a defer > > > > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > [ 1.176655] imx_pgc_power_down dispmix > > > > [ 1.181790] libphy: fec_enet_mii_bus: probed > > > > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > > > > ^^^ not sure what this is but had it back in my 5.10 solution as well > > > > so did not investigate > > > > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > [ 3.926936] imx_pgc_power_up dispmix > > > > [ 3.931109] imx_pgc_power_up mipi > > > > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > > > > 32e00000.lcdif on minor 0 > > > > ^^^ not clear why dispblk-lcdif got disabled here > > > > > > I've reproduced this. look like lcdif power-domains are not notified > > > ON. checking on the power-sequence for lcdif side. old patches from > > > Lucas on v5.13 seems working as expected. > > > > I've found the issue on lcdif atomic_enable, where the bus format is > > retrieved from NULL bus_state. Here is the diff for it. > > > > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > @@ -359,13 +359,14 @@ static void mxsfb_crtc_atomic_enable(struct > > drm_crtc *crtc, > > drm_crtc_vblank_on(crtc); > > > > /* If there is a bridge attached to the LCDIF, use its bus format */ > > - if (mxsfb->bridge) { > > + if (mxsfb->bridge && state) { > > bridge_state = > > drm_atomic_get_new_bridge_state(state, > > mxsfb->bridge); > > - bus_format = bridge_state->input_bus_cfg.format; > > + if (bridge_state) > > + bus_format = bridge_state->input_bus_cfg.format; > >' > > I believe this can be fixed via DSIM bridge. working on for those > > changes, the key challenges is to make the DSIM to work even for > > exynos, which indeed blocker for me to validate in hardware (i don't > > have DSI based exynos). > > > > Meanwhile, I have posed RFC for DSIM DTS changes, please check it here. > > https://patchwork.kernel.org/project/dri-devel/cover/20211111101456.584061-1-jagan@amarulasolutions.com/ > > > > Jagan, > > Thank you! This does resolve the hang on my end as well. I will look > at your 'arm64: imx8mm: Add MIPI DSI support' series. > > There was some discussion regarding giving up on trying to split up > the exynos driver such that it could work with IMX8MM vs just > duplicating it. I thought the recommendation was to duplicate it as it > wasn't clear if there was a way to split it out without breaking > current exynos DSI but I'll have to see if I can find the thread. The thread is here: https://lore.kernel.org/all/CAKMK7uF0B1TrtVX+9Whasz49quha_is+77z2wX3c06BsWRiPcQ@mail.gmail.com/ Duplicating seems to be the best way forward, because the Exynos driver exposes some special behavior (discussed earlier in the same thread), which isn't compatible with how bridges work. Michael
On Fri, Nov 12, 2021 at 2:12 PM Michael Tretter <m.tretter@pengutronix.de> wrote: > > On Thu, 11 Nov 2021 13:21:03 -0800, Tim Harvey wrote: > > On Thu, Nov 11, 2021 at 2:19 AM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > On Wed, Nov 10, 2021 at 11:58 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > > > > > > > > > [...] > > > > > > > > > > > > >> 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>; > > > > > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > > > > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > > > > > MXSFB_V4, but with overlays and those overlays have more registers > > > > > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > > > > > to see if it makes a difference? > > > > > > > > > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > > > > > > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > > > > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > > > > > ... just to make sure there is no confusion. > > > > > > > > > > > > [...] > > > > > > > > > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > > > > > >> + #address-cells = <1>; > > > > > > >> + #size-cells = <0>; > > > > > > >> + 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>; > > > > > > > > > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > > > > > samsung,burst-clock-frequency is really the DSI link clock which is > > > > > > panel/bridge specific ... but, why do we need to specify such policy in > > > > > > DT rather than have the panel/bridge drivers negotiate the best clock > > > > > > settings with DSIM bridge driver ? This should be something which should > > > > > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > > > > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > > > > > > > > > Also, are the DSIM bindings stable now ? > > > > > > > > > > Thanks Marek. > > > > > > > > > > No, there is no dsim driver yet. I'm not clear if there is still > > > > > dissagreement on if the drm/exynos driver can be split up or if a > > > > > whole new somewhat duplicate driver needs to be made. I know Jagan > > > > > also has a series he is working on that uses drm/exynos which I > > > > > believe he will share an update on in a day or so. > > > > > > > > > > I'm still using the work that Michael [1] and you [2] did a long time back. > > > > > > > > > > I had this solution working on a 5.10 kernel but it was based on the > > > > > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > > > > > issue I'm having is something not setup correctly with blk-ctl per my > > > > > dt settings or perhaps something missing from the blk-ctl driver like > > > > > Adam found [3] > > > > > > > > > > I am hanging at: > > > > > [ 1.064088] imx_pgc_power_up gpumix > > > > > [ 1.077506] imx_pgc_power_down gpumix > > > > > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > > [ 1.102682] imx_pgc_power_up dispmix > > > > > [ 1.106825] imx_pgc_power_up mipi > > > > > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > > > > > found, using dummy regulator > > > > > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > > > > > using dummy regulator > > > > > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > > > > > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > > > > > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > > > > > [ 1.143788] imx_pgc_power_down mipi > > > > > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > > [ 1.147316] imx_pgc_power_down dispmix > > > > > [ 1.155804] imx_pgc_power_up dispmix > > > > > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > > > > > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > > > > > ^^^ this is just a defer > > > > > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > > [ 1.176655] imx_pgc_power_down dispmix > > > > > [ 1.181790] libphy: fec_enet_mii_bus: probed > > > > > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > > > > > ^^^ not sure what this is but had it back in my 5.10 solution as well > > > > > so did not investigate > > > > > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > > [ 3.926936] imx_pgc_power_up dispmix > > > > > [ 3.931109] imx_pgc_power_up mipi > > > > > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > > > > > 32e00000.lcdif on minor 0 > > > > > ^^^ not clear why dispblk-lcdif got disabled here > > > > > > > > I've reproduced this. look like lcdif power-domains are not notified > > > > ON. checking on the power-sequence for lcdif side. old patches from > > > > Lucas on v5.13 seems working as expected. > > > > > > I've found the issue on lcdif atomic_enable, where the bus format is > > > retrieved from NULL bus_state. Here is the diff for it. > > > > > > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > > @@ -359,13 +359,14 @@ static void mxsfb_crtc_atomic_enable(struct > > > drm_crtc *crtc, > > > drm_crtc_vblank_on(crtc); > > > > > > /* If there is a bridge attached to the LCDIF, use its bus format */ > > > - if (mxsfb->bridge) { > > > + if (mxsfb->bridge && state) { > > > bridge_state = > > > drm_atomic_get_new_bridge_state(state, > > > mxsfb->bridge); > > > - bus_format = bridge_state->input_bus_cfg.format; > > > + if (bridge_state) > > > + bus_format = bridge_state->input_bus_cfg.format; > > >' > > > I believe this can be fixed via DSIM bridge. working on for those > > > changes, the key challenges is to make the DSIM to work even for > > > exynos, which indeed blocker for me to validate in hardware (i don't > > > have DSI based exynos). > > > > > > Meanwhile, I have posed RFC for DSIM DTS changes, please check it here. > > > https://patchwork.kernel.org/project/dri-devel/cover/20211111101456.584061-1-jagan@amarulasolutions.com/ > > > > > > > Jagan, > > > > Thank you! This does resolve the hang on my end as well. I will look > > at your 'arm64: imx8mm: Add MIPI DSI support' series. > > > > There was some discussion regarding giving up on trying to split up > > the exynos driver such that it could work with IMX8MM vs just > > duplicating it. I thought the recommendation was to duplicate it as it > > wasn't clear if there was a way to split it out without breaking > > current exynos DSI but I'll have to see if I can find the thread. > > The thread is here: > > https://lore.kernel.org/all/CAKMK7uF0B1TrtVX+9Whasz49quha_is+77z2wX3c06BsWRiPcQ@mail.gmail.com/ > > Duplicating seems to be the best way forward, because the Exynos driver > exposes some special behavior (discussed earlier in the same thread), which > isn't compatible with how bridges work. Not quite sure about it. Laurent and Inki had similar discussion and looking for common bridge [1]. [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210704090230.26489-7-jagan@amarulasolutions.com/ Jagan.
On Fri, Nov 12, 2021 at 02:28:30PM +0530, Jagan Teki wrote: > On Fri, Nov 12, 2021 at 2:12 PM Michael Tretter wrote: > > On Thu, 11 Nov 2021 13:21:03 -0800, Tim Harvey wrote: > > > On Thu, Nov 11, 2021 at 2:19 AM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > On Wed, Nov 10, 2021 at 11:58 PM Jagan Teki <jagan@amarulasolutions.com> wrote: > > > > > On Wed, Nov 10, 2021 at 2:24 AM Tim Harvey <tharvey@gateworks.com> wrote: > > > > > > On Tue, Nov 9, 2021 at 12:39 PM Marek Vasut <marex@denx.de> wrote: > > > > > > > On 11/9/21 8:35 PM, Adam Ford wrote: > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > >> 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>; > > > > > > > >> + compatible = "fsl,imx8mm-lcdif", "fsl,imx6sx-lcdif"; > > > > > > > > > > > > > > > > The compatible "imx6sx-lcdif" implies MXSFB_V6. FWICT, it is like > > > > > > > > MXSFB_V4, but with overlays and those overlays have more registers > > > > > > > > configured in the mxsfb_kms driver. Have you tried using imx28-lcdif > > > > > > > > to see if it makes a difference? > > > > > > > > > > > > > > Indeed, MX6SX has AS overlay plane support, MX{2,}8 does not. > > > > > > > > > > > > > > LCDIFv3 (as NXP calls it) in MX8MP is like LCDIFv6 (in MX6SX) with > > > > > > > slightly reordered register bits, but nothing like LCDIF rev3 (in MX23) > > > > > > > ... just to make sure there is no confusion. > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > >> + mipi_dsi: mipi_dsi@32e10000 { > > > > > > > >> + #address-cells = <1>; > > > > > > > >> + #size-cells = <0>; > > > > > > > >> + 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>; > > > > > > > > > > > > > > This 27 MHz is really IMX8MM_CLK_DSI_PHY_REF and > > > > > > > samsung,burst-clock-frequency is really the DSI link clock which is > > > > > > > panel/bridge specific ... but, why do we need to specify such policy in > > > > > > > DT rather than have the panel/bridge drivers negotiate the best clock > > > > > > > settings with DSIM bridge driver ? This should be something which should > > > > > > > be implemented in the DRM subsystem, not hard-coded in DT. These ad-hoc > > > > > > > samsung,*-clock-frequency properties shouldn't even be needed then. > > > > > > > > > > > > > > Also, are the DSIM bindings stable now ? > > > > > > > > > > > > Thanks Marek. > > > > > > > > > > > > No, there is no dsim driver yet. I'm not clear if there is still > > > > > > dissagreement on if the drm/exynos driver can be split up or if a > > > > > > whole new somewhat duplicate driver needs to be made. I know Jagan > > > > > > also has a series he is working on that uses drm/exynos which I > > > > > > believe he will share an update on in a day or so. > > > > > > > > > > > > I'm still using the work that Michael [1] and you [2] did a long time back. > > > > > > > > > > > > I had this solution working on a 5.10 kernel but it was based on the > > > > > > old unapproved IMX8MM blk-ctl and pd drivers. Therefore I believe the > > > > > > issue I'm having is something not setup correctly with blk-ctl per my > > > > > > dt settings or perhaps something missing from the blk-ctl driver like > > > > > > Adam found [3] > > > > > > > > > > > > I am hanging at: > > > > > > [ 1.064088] imx_pgc_power_up gpumix > > > > > > [ 1.077506] imx_pgc_power_down gpumix > > > > > > [ 1.097685] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > > > [ 1.102682] imx_pgc_power_up dispmix > > > > > > [ 1.106825] imx_pgc_power_up mipi > > > > > > [ 1.110232] imx-dsim-dsi 32e10000.mipi_dsi: supply vddcore not > > > > > > found, using dummy regulator > > > > > > [ 1.118760] imx-dsim-dsi 32e10000.mipi_dsi: supply vddio not found, > > > > > > using dummy regulator > > > > > > [ 1.127361] imx-dsim-dsi 32e10000.mipi_dsi: [drm] *ERROR* modalias > > > > > > failure on /soc@0/bus@32c00000/mipi_dsi@32e10000/port@0 > > > > > > [ 1.138676] imx8m_blk_ctrl_power_off dispblk-mipi-dsi > > > > > > [ 1.143788] imx_pgc_power_down mipi > > > > > > [ 1.145278] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > > > [ 1.147316] imx_pgc_power_down dispmix > > > > > > [ 1.155804] imx_pgc_power_up dispmix > > > > > > [ 1.159820] [drm:drm_bridge_attach] *ERROR* failed to attach bridge > > > > > > /soc@0/bus@32c00000/mipi_dsi@32e10000 to encoder None-34: -517 > > > > > > ^^^ this is just a defer > > > > > > [ 1.171789] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > > > [ 1.176655] imx_pgc_power_down dispmix > > > > > > [ 1.181790] libphy: fec_enet_mii_bus: probed > > > > > > [ 3.915806] panel-simple panel: Expected bpc in {6,8} but got: 0 > > > > > > ^^^ not sure what this is but had it back in my 5.10 solution as well > > > > > > so did not investigate > > > > > > [ 3.921912] imx8m_blk_ctrl_power_on dispblk-mipi-dsi > > > > > > [ 3.926936] imx_pgc_power_up dispmix > > > > > > [ 3.931109] imx_pgc_power_up mipi > > > > > > [ 3.935409] imx8m_blk_ctrl_power_on dispblk-lcdif > > > > > > [ 3.940547] imx8m_blk_ctrl_power_off dispblk-lcdif > > > > > > [ 3.945626] [drm] Initialized mxsfb-drm 1.0.0 20160824 for > > > > > > 32e00000.lcdif on minor 0 > > > > > > ^^^ not clear why dispblk-lcdif got disabled here > > > > > > > > > > I've reproduced this. look like lcdif power-domains are not notified > > > > > ON. checking on the power-sequence for lcdif side. old patches from > > > > > Lucas on v5.13 seems working as expected. > > > > > > > > I've found the issue on lcdif atomic_enable, where the bus format is > > > > retrieved from NULL bus_state. Here is the diff for it. > > > > > > > > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > > > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > > > > @@ -359,13 +359,14 @@ static void mxsfb_crtc_atomic_enable(struct > > > > drm_crtc *crtc, > > > > drm_crtc_vblank_on(crtc); > > > > > > > > /* If there is a bridge attached to the LCDIF, use its bus format */ > > > > - if (mxsfb->bridge) { > > > > + if (mxsfb->bridge && state) { > > > > bridge_state = > > > > drm_atomic_get_new_bridge_state(state, > > > > mxsfb->bridge); > > > > - bus_format = bridge_state->input_bus_cfg.format; > > > > + if (bridge_state) > > > > + bus_format = bridge_state->input_bus_cfg.format; > > > >' > > > > I believe this can be fixed via DSIM bridge. working on for those > > > > changes, the key challenges is to make the DSIM to work even for > > > > exynos, which indeed blocker for me to validate in hardware (i don't > > > > have DSI based exynos). > > > > > > > > Meanwhile, I have posed RFC for DSIM DTS changes, please check it here. > > > > https://patchwork.kernel.org/project/dri-devel/cover/20211111101456.584061-1-jagan@amarulasolutions.com/ > > > > > > > > > > Jagan, > > > > > > Thank you! This does resolve the hang on my end as well. I will look > > > at your 'arm64: imx8mm: Add MIPI DSI support' series. > > > > > > There was some discussion regarding giving up on trying to split up > > > the exynos driver such that it could work with IMX8MM vs just > > > duplicating it. I thought the recommendation was to duplicate it as it > > > wasn't clear if there was a way to split it out without breaking > > > current exynos DSI but I'll have to see if I can find the thread. > > > > The thread is here: > > > > https://lore.kernel.org/all/CAKMK7uF0B1TrtVX+9Whasz49quha_is+77z2wX3c06BsWRiPcQ@mail.gmail.com/ > > > > Duplicating seems to be the best way forward, because the Exynos driver > > exposes some special behavior (discussed earlier in the same thread), which > > isn't compatible with how bridges work. This seems to tbe the crux of the issue. I don't see what would be specific to Exynos there, those issues should be fixed globally. If the DRM bridge API can't support some use cases, it should be improved. > Not quite sure about it. Laurent and Inki had similar discussion and > looking for common bridge [1]. > > [1] https://patchwork.kernel.org/project/linux-arm-kernel/patch/20210704090230.26489-7-jagan@amarulasolutions.com/
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>; + 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>; + + lcdif_to_dsim: endpoint { + remote-endpoint = <&dsim_from_lcdif>; + }; + }; + }; + + mipi_dsi: mipi_dsi@32e10000 { + #address-cells = <1>; + #size-cells = <0>; + 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"; + + 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>;