IMX219: Adjust PLL settings based on the number of MIPI lanes

[peyton-howe]

Still can tweak the exact PLL settings, but this adds the overlays and driver support needed for 4-lane operation. I have tested on my Pi5 and run with both 4-lane and 2-lane cameras simultaneously.

[6by9]

Links to https://forums.raspberrypi.com/viewtopic.php?t=381663

For reference, driver changes and overlay changes need to be in separate patches.

The overlay can be an override on the existing imx219 overlay to reduce duplication. My diff (which I can't test) doing that was

diff --git a/arch/arm/boot/dts/overlays/imx219-overlay.dts b/arch/arm/boot/dts/overlays/imx219-overlay.dts
index 4c4bcd309a3d..3411d5bbd8ce 100644
--- a/arch/arm/boot/dts/overlays/imx219-overlay.dts
+++ b/arch/arm/boot/dts/overlays/imx219-overlay.dts
@@ -65,6 +65,21 @@ csi_ep: endpoint {
                };
        };
 
+       fragment@201 {
+               target = <&csi_ep>;
+               __dormant__ {
+                       data-lanes = <1 2 3 4>;
+               };
+       };
+
+       fragment@202 {
+               target = <&cam_endpoint>;
+               __dormant__ {
+                       data-lanes = <1 2 3 4>;
+                       link-frequencies =
+                               /bits/ 64 <363000000>;
+       };
+
        __overrides__ {
                rotation = <&cam_node>,"rotation:0";
                orientation = <&cam_node>,"orientation:0";
@@ -77,6 +92,7 @@ __overrides__ {
                       <&vcm>, "VANA-supply:0=", <&cam0_reg>;
                vcm = <&vcm>, "status=okay",
                      <&cam_node>,"lens-focus:0=", <&vcm>;
+               4lane = <0>, "+201+202";
        };
 };

[6by9]

You're not handling any failures from this table write.

TBH you may as well move the IMX219_REG_CSI_LANE_MODE register into the array and just return the value from cci_multi_reg_write, or pass &ret as the last argument to cci_write below.

[peyton-howe]

Applied your recommended changes in the latest commit

[pelwell]

There's a missing line (another closing brace and semicolon) in the proposed fragment 202, but otherwise it looks okay.

[peyton-howe]

Removed overlay changes from this PR and updated driver based on 6by9's suggestion

[pelwell]

Can you give us a Signed-off-by line for the changes in your PRs (this and #6580)? Mine would be: Signed-off-by: Phil Elwell <[email protected]>

[peyton-howe]

Done for both!

[pelwell]

This change is spurious and makes checkpatch sad.

[peyton-howe]

Should be fixed now :)

[pelwell]

Any comments, @6by9?

[6by9]

I'd hoped to test with a module, but it got stuck in customs (hopefully arriving tomorrow).

Commit message could be fuller, and include Fixes: ceddfd4493b3 ("media: i2c: imx219: Support four-lane operation"), but I can do that when upstreaming it.

Do the pixel rate numbers work out for frame rate control?
That upstream commit changes the pixel rate to 280.8MPix/s (from 184.2) and link frequency of 363MHz (from 456MHz). Both those numbers come from section "9-2 clock setting example" in the datasheet (see https://github.com/rellimmot/Sony-IMX219-Raspberry-Pi-V2-CMOS/blob/master/RASPBERRY%20PI%20CAMERA%20V2%20DATASHEET%20IMX219PQH5_7.0.0_Datasheet_XXX.PDF).

sohonomura2020's post at https://forums.raspberrypi.com/viewtopic.php?p=2283325#p2283325 implies the new IMX219_REG_PLL_VT_MPY should be 0x57 = 87 and IMX219_REG_PLL_OP_MPY should be 0x5a = 90. Your patch has 88 and 91.

My maths says that IMX219_REG_PLL_OP_MPY = 91 gives a link rate of 364MHz, whilst 90 gives a link rate of 360MHz. Neither is the driver value of 363MHz.
Likewise for IMX219_REG_PLL_VT_MPY, =88 gives a pixel rate of 281.6MPix/s, whilst 87 gives a pixel rate of 278.4MPix/s. Neither is the driver value of 280.8MPix/s.
So whilst your numbers will improve matters, they still look slightly off. Frame rates and exposure times will likewise be slightly wrong.

The example does say EXCK_FREQ = 12MHz.
702/12 = 58.5, so I think the only way that the example timings can be achieved is by INCK=12MHz, Pre-Div1 = 2, and then PLL_VT_MPY = 117.
We have INCK = 24MHz. There is no Pre-Div1 option of /4 listed (although it is defined as an 8 bit register, so might be worth a quick test), so I don't think the numbers can be achieved.

Continuing the search, registers 0x110A/B are listed as max_pre_pll_clk_div / "Maximum Pre PLL divider value" with a value of 0x0D (14 decimal), so perhaps /4 is permitted. That would be worth a test, as amending the link frequency is a pain (we'd have to log a warning for the approximation if asked for the previously "supported" value).

[peyton-howe]

I tried setting the pre-dividers to /4 and it didn't work, the frame rates are way off. What would you suggest we do here?

[6by9]

I tried setting the pre-dividers to /4 and it didn't work, the frame rates are way off. What would you suggest we do here?

My 4 lane module arrived today. I'll try and have a play tomorrow to see what we can achieve.
It'd be interesting to know if registers 0x110A/B do report a max pre-pll divider of 0x0d, or is that a datasheet error.

If the specified rates can't be achieved, then they'll have to be altered, and a WARN added if the unsupported rate is requested.

[6by9]

The docs are unclear here.

The IMX219PQH5-C has the function that automatically set Pre-Div1 and Pre-Div2 by setting the register
by setting the register of EXCK_FREQ, case of changing INCK frequency

For a 24MHz EXCK_FREQ it'll set div 3, hence the comment in the driver source /* 0x03 = AUTO set */
Registers 0x0304 and 0x0305 are then listed as being RW and re-timed at VSYNC.
We are writing them after EXCK_FREQ, so one would hope they'd be accepted, but is it on the write of EXCK_FREQ that the registers are updated, or some other point?

However my findings seem to match yours - the value can't be changed.
With numbers recomputed for a /4, I just get the framerate running 1.333 (4/3) times faster that it should be :-(
Trying to fiddle with dividers 0x0303 or 0x030b also seems to fail to make any difference.
...
Ah, it's only whilst streaming some values can't be changed.
If I send i2ctransfer -f -y 10 w3@0x10 0x01 0x00 0x00 w3@0x10 0x03 0x03 0x02 w3@0x10 0x01 0x00 0x01 to stop streaming, change 0x0303, and resuming streaming, then I get the frame rate change. Actually that is fair as the docs don't list it as having a re-time point, unlike many of the others.

I don't believe I have any further information on the IMX219 PLL configuration, and it's looking like the exact desired rates are just unachievable with a 24MHz clock. Short of a minor miracle beating the PLL into submission, I think we'll have to change the link frequencies.

[6by9]

#6615 is an updated version of this (on a 6.12 kernel) that I intend to upstream. I'll sort a 6.6. backport in due course.

[6by9]

Closing as superceded by #6618 and #6615