diff mbox series

arm64: dts: imx8mp: Add CSIS DT nodes

Message ID 20220616161643.22867-1-laurent.pinchart@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series arm64: dts: imx8mp: Add CSIS DT nodes | expand

Commit Message

Laurent Pinchart June 16, 2022, 4:16 p.m. UTC
Add DT nodes for the two CSI-2 receivers of the i.MX8MP.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
This patch depends on the DT bindings submitted in [1], for which I plan
to submit a pull request for v5.20.

[1] https://lore.kernel.org/linux-media/83e27382-6f97-015f-2ee1-f43820967093@linaro.org/T/#u
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++
 1 file changed, 60 insertions(+)


base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

Comments

Alexander Stein June 17, 2022, 6:43 a.m. UTC | #1
Hello Laurent,

thanks for ths and the other MIPI CSI-2 related patches.

Am Donnerstag, 16. Juni 2022, 18:16:43 CEST schrieb Laurent Pinchart:
> Add DT nodes for the two CSI-2 receivers of the i.MX8MP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> This patch depends on the DT bindings submitted in [1], for which I plan
> to submit a pull request for v5.20.
> 
> [1]
> https://lore.kernel.org/linux-media/83e27382-6f97-015f-2ee1-f43820967093@li
> naro.org/T/#u ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index
> d9542dfff83f..c8ed206b7f41 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1063,6 +1063,66 @@ media_blk_ctrl: blk-ctrl@32ec0000 {
>  				#power-domain-cells = <1>;
>  			};
> 
> +			mipi_csi_0: csi@32e40000 {
> +				compatible = "fsl,imx8mp-mipi-
csi2", "fsl,imx8mm-mipi-csi2";
> +				reg = <0x32e40000 0x10000>;
> +				interrupts = <GIC_SPI 17 
IRQ_TYPE_LEVEL_HIGH>;
> +				clock-frequency = <500000000>;

According to datasheet (IMX8MPIEC Rev 1, Table 1, Subsystem "MIPI Interface") 
"MIPI CSI1" supports up to 500MHz only in single camera use and overdrive 
mode. In normal mode only 400MHz are supported. For dual camera usage only up 
to 266MHz is supported. Apparently this is when using ISP, things might be 
different when using ISI. I'm hesitating specifying the overdrive mode 
frequency here. Most users, most probably using normal mode, would have 
requiring them to adjust this.
For dual camera this is even as low as 266MHz, but IMHO this is a special 
case.

> +				clocks = <&clk 
IMX8MP_CLK_MEDIA_APB_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_AXI_ROOT>;
> +				clock-names = "pclk", "wrap", 
"phy", "axi";
> +				assigned-clocks = <&clk 
IMX8MP_CLK_MEDIA_CAM1_PIX>;
> +				assigned-clock-parents = <&clk 
IMX8MP_SYS_PLL2_1000M>;
> +				assigned-clock-rates = 
<500000000>;
> +				power-domains = <&media_blk_ctrl 
IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +					};
> +				};
> +			};
> +
> +			mipi_csi_1: csi@32e50000 {
> +				compatible = "fsl,imx8mp-mipi-
csi2", "fsl,imx8mm-mipi-csi2";
> +				reg = <0x32e50000 0x10000>;
> +				interrupts = <GIC_SPI 80 
IRQ_TYPE_LEVEL_HIGH>;
> +				clock-frequency = <266000000>;

For single camera usage this can even go as high as 277MHz. 266MHz is only for 
dual camera use.

Best regards,
Alexander

> +				clocks = <&clk 
IMX8MP_CLK_MEDIA_APB_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> +					 <&clk 
IMX8MP_CLK_MEDIA_AXI_ROOT>;
> +				clock-names = "pclk", "wrap", 
"phy", "axi";
> +				assigned-clocks = <&clk 
IMX8MP_CLK_MEDIA_CAM2_PIX>;
> +				assigned-clock-parents = <&clk 
IMX8MP_SYS_PLL2_1000M>;
> +				assigned-clock-rates = 
<266000000>;
> +				power-domains = <&media_blk_ctrl 
IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
> +				status = "disabled";
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +					};
> +				};
> +			};
> +
>  			hsio_blk_ctrl: blk-ctrl@32f10000 {
>  				compatible = "fsl,imx8mp-hsio-blk-
ctrl", "syscon";
>  				reg = <0x32f10000 0x24>;
> 
> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
Laurent Pinchart June 17, 2022, 10:31 p.m. UTC | #2
Hi Alexander,

On Fri, Jun 17, 2022 at 08:43:53AM +0200, Alexander Stein wrote:
> Hello Laurent,
> 
> thanks for ths and the other MIPI CSI-2 related patches.
> 
> Am Donnerstag, 16. Juni 2022, 18:16:43 CEST schrieb Laurent Pinchart:
> > Add DT nodes for the two CSI-2 receivers of the i.MX8MP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > This patch depends on the DT bindings submitted in [1], for which I plan
> > to submit a pull request for v5.20.
> > 
> > [1] https://lore.kernel.org/linux-media/83e27382-6f97-015f-2ee1-f43820967093@linaro.org/T/#u
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index
> > d9542dfff83f..c8ed206b7f41 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1063,6 +1063,66 @@ media_blk_ctrl: blk-ctrl@32ec0000 {
> >  				#power-domain-cells = <1>;
> >  			};
> > 
> > +			mipi_csi_0: csi@32e40000 {
> > +				compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
> > +				reg = <0x32e40000 0x10000>;
> > +				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> > +				clock-frequency = <500000000>;
> 
> According to datasheet (IMX8MPIEC Rev 1, Table 1, Subsystem "MIPI Interface") 
> "MIPI CSI1" supports up to 500MHz only in single camera use and overdrive 
> mode. In normal mode only 400MHz are supported. For dual camera usage only up 
> to 266MHz is supported.

I wasn't aware of that, thank you for the information.

> Apparently this is when using ISP, things might be 
> different when using ISI.

Table 13 documents the maximum frequencies of clocks
MEDIA_CAM1_PIX_CLK_ROOT and MEDIA_CAM2_PIX_CLK_ROOT to be 400/500MHz
(normal/overdrive) and 277/277MHz respectively, so I'd say this affects
the ISI too. I wonder what causes the 266MHz constraint for dual camera
mode.

There's also a constraint of at most 375 MPixel/s aggregate performance
for the two ISP instances, but I don't know if that's due to the memory
bandwidth, or if it is on the input side in which case it may include
blanking and translate directly to clock frequencies. If I had to guess,
I'd say the former.

> I'm hesitating specifying the overdrive mode 
> frequency here. Most users, most probably using normal mode, would have 
> requiring them to adjust this.
> For dual camera this is even as low as 266MHz, but IMHO this is a special 
> case.

I agree, we should at least lower the frequency to 400MHz here. Given
that the frequency limit depends on whether one or two cameras are used,
I'm actually tempted to either specify the worst case (2x 266MHz), or
even drop the clock-frequency completely, forcing users to think about
what they need. The driver however silently falls back to a default
frequency of 166MHz when the property isn't set, so developers won't
necessarily immediately notice that something is wrong, or why.

Should I specify 400 MHz and 266 MHz here, or go for the safer option of
266 MHz and 266 MHz ?

> > +				clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
> > +				clock-names = "pclk", "wrap", "phy", "axi";
> > +				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>;
> > +				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
> > +				assigned-clock-rates = <500000000>;
> > +				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
> > +				status = "disabled";
> > +
> > +				ports {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					port@0 {
> > +						reg = <0>;
> > +					};
> > +
> > +					port@1 {
> > +						reg = <1>;
> > +					};
> > +				};
> > +			};
> > +
> > +			mipi_csi_1: csi@32e50000 {
> > +				compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
> > +				reg = <0x32e50000 0x10000>;
> > +				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
> > +				clock-frequency = <266000000>;
> 
> For single camera usage this can even go as high as 277MHz. 266MHz is only for 
> dual camera use.
> 
> > +				clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> > +					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
> > +				clock-names = "pclk", "wrap", "phy", "axi";
> > +				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM2_PIX>;
> > +				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
> > +				assigned-clock-rates = <266000000>;
> > +				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
> > +				status = "disabled";
> > +
> > +				ports {
> > +					#address-cells = <1>;
> > +					#size-cells = <0>;
> > +
> > +					port@0 {
> > +						reg = <0>;
> > +					};
> > +
> > +					port@1 {
> > +						reg = <1>;
> > +					};
> > +				};
> > +			};
> > +
> >  			hsio_blk_ctrl: blk-ctrl@32f10000 {
> >  				compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
> >  				reg = <0x32f10000 0x24>;
> > 
> > base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
Alexander Stein June 20, 2022, 12:21 p.m. UTC | #3
Am Samstag, 18. Juni 2022, 00:31:10 CEST schrieb Laurent Pinchart:
> Hi Alexander,
> 
> On Fri, Jun 17, 2022 at 08:43:53AM +0200, Alexander Stein wrote:
> > Hello Laurent,
> > 
> > thanks for ths and the other MIPI CSI-2 related patches.
> > 
> > Am Donnerstag, 16. Juni 2022, 18:16:43 CEST schrieb Laurent Pinchart:
> > > Add DT nodes for the two CSI-2 receivers of the i.MX8MP.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > This patch depends on the DT bindings submitted in [1], for which I plan
> > > to submit a pull request for v5.20.
> > > 
> > > [1]
> > > https://lore.kernel.org/linux-media/83e27382-6f97-015f-2ee1-f4382096709
> > > 3@linaro.org/T/#u ---
> > > 
> > >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 60 +++++++++++++++++++++++
> > >  1 file changed, 60 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index
> > > d9542dfff83f..c8ed206b7f41 100644
> > > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > > @@ -1063,6 +1063,66 @@ media_blk_ctrl: blk-ctrl@32ec0000 {
> > > 
> > >  				#power-domain-cells = <1>;
> > >  			
> > >  			};
> > > 
> > > +			mipi_csi_0: csi@32e40000 {
> > > +				compatible = "fsl,imx8mp-mipi-
csi2", "fsl,imx8mm-mipi-csi2";
> > > +				reg = <0x32e40000 0x10000>;
> > > +				interrupts = <GIC_SPI 17 
IRQ_TYPE_LEVEL_HIGH>;
> > > +				clock-frequency = <500000000>;
> > 
> > According to datasheet (IMX8MPIEC Rev 1, Table 1, Subsystem "MIPI
> > Interface") "MIPI CSI1" supports up to 500MHz only in single camera use
> > and overdrive mode. In normal mode only 400MHz are supported. For dual
> > camera usage only up to 266MHz is supported.
> 
> I wasn't aware of that, thank you for the information.

I wasn't either ;-) Just stumbled across when looking why mpi_csi_1 had only 
266MHz.

> > Apparently this is when using ISP, things might be
> > different when using ISI.
> 
> Table 13 documents the maximum frequencies of clocks
> MEDIA_CAM1_PIX_CLK_ROOT and MEDIA_CAM2_PIX_CLK_ROOT to be 400/500MHz
> (normal/overdrive) and 277/277MHz respectively, so I'd say this affects
> the ISI too. I wonder what causes the 266MHz constraint for dual camera
> mode.
> 
> There's also a constraint of at most 375 MPixel/s aggregate performance
> for the two ISP instances, but I don't know if that's due to the memory
> bandwidth, or if it is on the input side in which case it may include
> blanking and translate directly to clock frequencies. If I had to guess,
> I'd say the former.

I do not know either. A colleague suggested it might be due to DMA bandwidth 
limitation, but this is pure speculation.

> > I'm hesitating specifying the overdrive mode
> > frequency here. Most users, most probably using normal mode, would have
> > requiring them to adjust this.
> > For dual camera this is even as low as 266MHz, but IMHO this is a special
> > case.
> 
> I agree, we should at least lower the frequency to 400MHz here. Given
> that the frequency limit depends on whether one or two cameras are used,
> I'm actually tempted to either specify the worst case (2x 266MHz), or
> even drop the clock-frequency completely, forcing users to think about
> what they need. The driver however silently falls back to a default
> frequency of 166MHz when the property isn't set, so developers won't
> necessarily immediately notice that something is wrong, or why.
> 
> Should I specify 400 MHz and 266 MHz here, or go for the safer option of
> 266 MHz and 266 MHz ?

If the property is dropped and 166MHz is selected by default, then a dev_info 
should be added at least. I have no experience how common a dual camera setup 
is, so I tend to the, probably, more common single camera setup.
I was not able to get the same information for imx8mm, but maybe these limits 
or hint about the difference for single/dual camera can be put into the DT 
bindings.

Best regards,
Alexander

> > > +				clocks = <&clk 
IMX8MP_CLK_MEDIA_APB_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_AXI_ROOT>;
> > > +				clock-names = "pclk", "wrap", 
"phy", "axi";
> > > +				assigned-clocks = <&clk 
IMX8MP_CLK_MEDIA_CAM1_PIX>;
> > > +				assigned-clock-parents = <&clk 
IMX8MP_SYS_PLL2_1000M>;
> > > +				assigned-clock-rates = 
<500000000>;
> > > +				power-domains = <&media_blk_ctrl 
IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
> > > +				status = "disabled";
> > > +
> > > +				ports {
> > > +					#address-cells = <1>;
> > > +					#size-cells = <0>;
> > > +
> > > +					port@0 {
> > > +						reg = <0>;
> > > +					};
> > > +
> > > +					port@1 {
> > > +						reg = <1>;
> > > +					};
> > > +				};
> > > +			};
> > > +
> > > +			mipi_csi_1: csi@32e50000 {
> > > +				compatible = "fsl,imx8mp-mipi-
csi2", "fsl,imx8mm-mipi-csi2";
> > > +				reg = <0x32e50000 0x10000>;
> > > +				interrupts = <GIC_SPI 80 
IRQ_TYPE_LEVEL_HIGH>;
> > > +				clock-frequency = <266000000>;
> > 
> > For single camera usage this can even go as high as 277MHz. 266MHz is only
> > for dual camera use.
> > 
> > > +				clocks = <&clk 
IMX8MP_CLK_MEDIA_APB_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
> > > +					 <&clk 
IMX8MP_CLK_MEDIA_AXI_ROOT>;
> > > +				clock-names = "pclk", "wrap", 
"phy", "axi";
> > > +				assigned-clocks = <&clk 
IMX8MP_CLK_MEDIA_CAM2_PIX>;
> > > +				assigned-clock-parents = <&clk 
IMX8MP_SYS_PLL2_1000M>;
> > > +				assigned-clock-rates = 
<266000000>;
> > > +				power-domains = <&media_blk_ctrl 
IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
> > > +				status = "disabled";
> > > +
> > > +				ports {
> > > +					#address-cells = <1>;
> > > +					#size-cells = <0>;
> > > +
> > > +					port@0 {
> > > +						reg = <0>;
> > > +					};
> > > +
> > > +					port@1 {
> > > +						reg = <1>;
> > > +					};
> > > +				};
> > > +			};
> > > +
> > > 
> > >  			hsio_blk_ctrl: blk-ctrl@32f10000 {
> > >  			
> > >  				compatible = "fsl,imx8mp-hsio-blk-
ctrl", "syscon";
> > >  				reg = <0x32f10000 0x24>;
> > > 
> > > base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index d9542dfff83f..c8ed206b7f41 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1063,6 +1063,66 @@  media_blk_ctrl: blk-ctrl@32ec0000 {
 				#power-domain-cells = <1>;
 			};
 
+			mipi_csi_0: csi@32e40000 {
+				compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
+				reg = <0x32e40000 0x10000>;
+				interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency = <500000000>;
+				clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_CAM1_PIX_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
+				clock-names = "pclk", "wrap", "phy", "axi";
+				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM1_PIX>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
+				assigned-clock-rates = <500000000>;
+				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_1>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
+			mipi_csi_1: csi@32e50000 {
+				compatible = "fsl,imx8mp-mipi-csi2", "fsl,imx8mm-mipi-csi2";
+				reg = <0x32e50000 0x10000>;
+				interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency = <266000000>;
+				clocks = <&clk IMX8MP_CLK_MEDIA_APB_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_CAM2_PIX_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF_ROOT>,
+					 <&clk IMX8MP_CLK_MEDIA_AXI_ROOT>;
+				clock-names = "pclk", "wrap", "phy", "axi";
+				assigned-clocks = <&clk IMX8MP_CLK_MEDIA_CAM2_PIX>;
+				assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>;
+				assigned-clock-rates = <266000000>;
+				power-domains = <&media_blk_ctrl IMX8MP_MEDIABLK_PD_MIPI_CSI2_2>;
+				status = "disabled";
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+					};
+
+					port@1 {
+						reg = <1>;
+					};
+				};
+			};
+
 			hsio_blk_ctrl: blk-ctrl@32f10000 {
 				compatible = "fsl,imx8mp-hsio-blk-ctrl", "syscon";
 				reg = <0x32f10000 0x24>;