Re: (EXT) Re: (EXT) [PATCH] arm64: dts: imx8mp: Add CSIS DT nodes

[Alexander Stein <alexander.stein@ew.tq-group.com>]

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





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel