[PATCH] arm64: dts: imx8mp: Align both CSI2 pixel clock

[Alexander Stein]

Hi everyone,

Am Samstag, 6. April 2024, 04:58:39 CEST schrieb Marek Vasut:
> On 4/5/24 11:04 PM, Adam Ford wrote:
> > On Fri, Apr 5, 2024 at 3:43 PM Laurent Pinchart
> > <laurent.pinchart at ideasonboard.com> wrote:
> >>
> >> Hi Marek,
> >>
> >> (CC'ing Adam)
> >>
> >> Thank you for the patch.
> >>
> >> On Fri, Apr 05, 2024 at 10:22:26PM +0200, Marek Vasut wrote:
> >>> Configure both CSI2 assigned-clock-rates the same way.
> >>> There does not seem to be any reason for keeping the
> >>> two CSI2 pixel clock set to different frequencies.
> >>
> >> There's an issue when using two cameras concurrently. This has been
> >> discussed some time ago on the linux-media mailing list, see [1]. Adam
> >> knows more than I do on this topic.
> >>
> >> [1] https://lore.kernel.org/linux-media/CAHCN7x+kymRGO2kxvN2=zLiqRjfTc3hdf3VdNVkWjsW3La0bnA@mail.gmail.com/
> >>
> >>> Signed-off-by: Marek Vasut <marex at denx.de>
> >>> ---
> >>> Cc: Conor Dooley <conor+dt at kernel.org>
> >>> Cc: Fabio Estevam <festevam at gmail.com>
> >>> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>
> >>> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>> Cc: Paul Elder <paul.elder at ideasonboard.com>
> >>> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>
> >>> Cc: Rob Herring <robh at kernel.org>
> >>> Cc: Sascha Hauer <s.hauer at pengutronix.de>
> >>> Cc: Shawn Guo <shawnguo at kernel.org>
> >>> Cc: devicetree at vger.kernel.org
> >>> Cc: imx at lists.linux.dev
> >>> Cc: linux-arm-kernel at lists.infradead.org
> >>> ---
> >>>   arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> index 1bb96e96639f2..2e9ce0c3a9815 100644
> >>> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> >>> @@ -1703,7 +1703,7 @@ mipi_csi_1: csi at 32e50000 {
> >>>                                                  <&clk IMX8MP_CLK_MEDIA_MIPI_PHY1_REF>;
> >>>                                assigned-clock-parents = <&clk IMX8MP_SYS_PLL2_1000M>,
> >>>                                                         <&clk IMX8MP_CLK_24M>;
> >>> -                             assigned-clock-rates = <266000000>;
> >>> +                             assigned-clock-rates = <500000000>;
> > 
> > I am traveling, so I don't have the technical documents in front of
> > me, but I beleive this is an over-drive speed, and 400MHz would be the
> > single clock, standard rate.  I created an imx8mm-overdrive and
> > imx8mn-overdrive dtsi file to let users who operate in overdrive mode
> > to update their clocks in one place.
> > 
> > I also think this goes down if the user is running two cameras instead
> > of one.  I re-read the old thread, and it's coming back to me, but
> > until I can get settled into my hotel in Germany, I won't have time to
> > review.  I think the original idea was to use the lowest, conservative
> > value with the idea that people can tweak their clock settings if
> > they're only running one and if they are running in over-drive mode.
> 
> MX8MPCEC does indeed read 400 MHz regular, 500 MHz overdrive.
> 
> Shall we align both CSI2 ports to 400 MHz ? Currently they are one 500 
> MHz and the other 266 MHz .

No, that won't do. The (industrial products) datasheet says (Table 1):
* Single camera on CSI1: 400/500 MHz in normal/overdrive mode
* Single camera on CSI2: 277 MHz
* Dual camera on CSI1 & CSI2: 266 MHz

Assuming you need CSI2 more likely in a dual camera setup only, defaulting to
266MHz seems sensible to me.

Best regards,
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/