Hi Adam,
(CC'ing Kieran)
On Tue, Nov 28, 2023 at 09:17:51PM -0600, Adam Ford wrote:
> On Mon, Apr 17, 2023 at 1:01 AM Laurent Pinchart wrote:
> >
> > Add DT nodes for the two CSI-2 receivers of the i.MX8MP.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
>
> Laurent,
>
> Sorry to dig up an old thread, but I have a concern about the clock
> ratings and nominal mode vs overdrive mode. I started investigating
> the different data sheets amongst the various imx8m[mnp] families to
> make the default device trees run at nominal mode while also creating
> a separate dtsi file with settings for overdrive so boards who use it
> can include them without having to duplicate the clock settings for
> everyone who supports overdrive.
>
> > 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 2dd60e3252f3..2a374a4c14a2 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1239,6 +1239,66 @@ ldb_lvds_ch1: endpoint {
> > };
> > };
> >
> > + 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>;
>
> According to Rev 2.1 of the Data sheet (IMX8MPCEC), dated July 2023,
> 500MHz is listed as single-camera, overdrive mode. Single-camera,
> nominal mode is 400MHz, but there is more...
> If configured for dual cameras, both CSI can only support up to
> 266MHz, but we have partially configured both albeit without the
> actual camera sensors connected.
>
> > + 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>;
>
> 266MHz is correct for dual camera, but in single camera, the second
> CSI is capable of 277MHz.
>
> At a minimum, I'd like to fix the overdrive frequency to nominal, but
> since we're plumbing in both cameras, I wonder if it would be better
> to run both at 266MHz with a note on CSI0 that states it could run at
> 400 or 500 if the second CSI is disabled and a note on the second CSI
> that it could run at 277 when the first one is disabled? What are your
> thoughts?
My thoughts is that this all should be selected at runtime, based on how
many cameras are used. That won't be trivial to do though :-S Kieran,
you've been working with two cameras, any opinion ?
> > + 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>;
> > + };
> > + };
> > + };
> > +
> > pcie_phy: pcie-phy@32f00000 {
> > compatible = "fsl,imx8mp-pcie-phy";
> > reg = <0x32f00000 0x10000>;
--
Regards,
Laurent Pinchart
