Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IMX477 improvements #6208

Draft
wants to merge 8 commits into
base: rpi-6.6.y
Choose a base branch
from
Draft

Conversation

6by9
Copy link
Contributor

@6by9 6by9 commented Jun 5, 2024

Pushing for backup and @naushir 's reference

Dropping min VBLANK to 1 improves max frame times slightly.

pi@bookworm64:~ $ rpicam-hello --list-cameras
Available cameras
-----------------
0 : imx477 [4056x3040 12-bit RGGB] (/base/axi/pcie@120000/rp1/i2c@80000/imx477@1a)
    Modes: 'SRGGB10_CSI2P' : 1332x990 [127.21 fps - (696, 528)/2664x1980 crop]
           'SRGGB12_CSI2P' : 2028x1080 [60.99 fps - (0, 440)/4056x2160 crop]
                             2028x1520 [43.35 fps - (0, 0)/4056x3040 crop]
                             4056x3040 [11.51 fps - (0, 0)/4056x3040 crop]

Now working on doubling the link frequency. That won't quite get us 4k30, but adding a cropped mode would (just going down to 16:9 aspect may be sufficient).

I've also sussed the runes to switch any mode from between 10 and 12 bit. Computing the min HBLANK in each case is going to be tricky, so they may end up being 2 hardcoded values.
(I'm assuming libcamera will choose the higher bit depth if it can achieve the frame rate desired, so offering either depth is reasonable)

@6by9
Copy link
Contributor Author

6by9 commented Jun 7, 2024

Added a 16:9 cropped mode, and option of 10 or 12 readout for all modes.

pi@bookworm64:~ $ rpicam-hello --list-cameras
Available cameras
-----------------
0 : imx477 [4056x3040 12-bit RGGB] (/base/axi/pcie@120000/rp1/i2c@80000/imx477@1a)
    Modes: 'SRGGB10_CSI2P' : 1332x990 [254.45 fps - (696, 528)/2664x1980 crop]
                             2028x1080 [146.41 fps - (0, 440)/4056x2160 crop]
                             2028x1520 [104.05 fps - (0, 0)/4056x3040 crop]
                             4056x2160 [38.87 fps - (0, 0)/4056x3480 crop]
                             4056x3040 [27.62 fps - (0, 0)/4056x3040 crop]
           'SRGGB12_CSI2P' : 1332x990 [212.04 fps - (696, 528)/2664x1980 crop]
                             2028x1080 [122.00 fps - (0, 440)/4056x2160 crop]
                             2028x1520 [86.70 fps - (0, 0)/4056x3040 crop]
                             4056x2160 [32.39 fps - (0, 0)/4056x3480 crop]
                             4056x3040 [23.02 fps - (0, 0)/4056x3040 crop]

It does currently need a manual dtoverlay=imx477,pi5 to enable the increased link frequency.
The last patch hasn't got a SoB as I need to revisit dropping that default frame rate function. It makes life ugly with the mixed 10/12 bit, so doing so is very tempting.

I have seen coloured sparkles on some images, particularly when dark, however I'm not certain it's not my HDMI->DVI lead at fault. Testing needed on a known good setup.

There is an open question as to whether we accept 1.8Gbit/s even though that is out of spec for RP1, or scale it back to 1.5Gbit/s. And do we need to look at EMC implications?

@6by9 6by9 force-pushed the rpi-6.6.y-imx477 branch 2 times, most recently from 3107f42 to 2bafdbc Compare June 7, 2024 15:11
@njhollinghurst
Copy link
Contributor

Perhaps we could enable the out-of-spec modes using a module or DT parameter? It should be disabled by default to make it clear they are not generally supported.

(Unsupported because that they might work on some devices and not others; or might stop working on a new board revision; or work with certain type or length of camera cable; or have EMC issues...)

@6by9
Copy link
Contributor Author

6by9 commented Jun 10, 2024

Perhaps we could enable the out-of-spec modes using a module or DT parameter? It should be disabled by default to make it clear they are not generally supported.

(Unsupported because that they might work on some devices and not others; or might stop working on a new board revision; or work with certain type or length of camera cable; or have EMC issues...)

It is already - you have to enable it with dtoverlay=imx477,pi5. (Name of override open to discussion).
Camera auto detection by the firmware will currently never enable it.

@njhollinghurst
Copy link
Contributor

Ah. I didn't spot that. Though possibly it needs a scarier name...

@6by9
Copy link
Contributor Author

6by9 commented Jun 10, 2024

Name open to debate.
I haven't tried it on Pi0-4, but I doubt it'll cope at 1.8Gbit/s. The vc4 ISP certainly won't keep up at that rate.

I'll drop SM and DP an email to get guidance on EMC for this change. If it's feasible to make this the default for Pi5 then I don't see why we shouldn't, but it needs some more extensive testing first.

@6by9
Copy link
Contributor Author

6by9 commented Jun 10, 2024

I had a brief discussion with both SM and DP.
It'd need a test, but increasing frequency is unlikely to cause EMC problems in and of itself. It's the lower frequencies that are tougher.

Suggestion of hacking the driver to increase the clock by 10% (easy to do) and then stick it in the environmental test chamber to test it when both hot and cold. If that is solid, then we can have reasonable confidence that it'll work on the vast majority of boards.

We are going to have to test with the various lengths of official camera cables. Those who use non-impedance matched FFCs can be advised to use better cables or drop the override.

@njhollinghurst
Copy link
Contributor

D-PHY driver will currently log an error, and pin its timings (for e.g. clock recovery and entry/exit detection) for the 1350-1500 Mbps range.

With 10bpp and cropping can we not achieve 4kp30 at <= 1.5GHz?

@6by9
Copy link
Contributor Author

6by9 commented Jun 11, 2024

D-PHY driver will currently log an error, and pin its timings (for e.g. clock recovery and entry/exit detection) for the 1350-1500 Mbps range.

With 10bpp and cropping can we not achieve 4kp30 at <= 1.5GHz?

We could (32.39fps for cropped 10bit, 26.99fps in 12bit), but I just went for the simple option of reducing IOP_SYCK_DIV from 2 to 1 which doubles the CSI clock.
Altering IOP_PLL_MPY (0x030e) would allow us to go for a lower frequency if that was felt necessary, and that shouldn't affect any of the pixel array timings as it's still only the CSI2 PLL.

@6by9
Copy link
Contributor Author

6by9 commented Jun 11, 2024

Issue reported with the 2028x1080 and 2028x1520 modes with 10 bit readout - we get totally white frames.

It's only true at the new 1.8Gbit/s link frequency, so it's likely a duplicated register setting in the mode table vs programmatic configuration.

@6by9
Copy link
Contributor Author

6by9 commented Jun 18, 2024

Discussion with GSH says to keep to the 1.5Gbit/s spec of RP1, so this will want a littler rework to allow alternate PLL multipliers in the MIPI clock path.

We may as well make it more generic so that MPP_IOP is computed based on the link frequency so any multiple of 12MHz can be used
(Current configuration is 24MHz input clock with Pre_Div_IOP = /2. Multiplier is x150 to give 1.8GHz. Finally Div_IOP_Px = /2 for the 450MHz / 900Mbit/s mode, or /1 for 1.8Gbit/s, giving us the desired pixel rates. Permitted PLL range is 1.2 - 2.1GHz, so it will need to set Div_IOP_Px programmatically too. That does leave a gap between 1.05Gbit/s and 1.2Gbit/s that can't be achieved, but I'm not going to look at trying to solve that. Someone else can sort out totally generic PLL configuration if they really want to)

@sandyol55
Copy link

Issue reported with the 2028x1080 and 2028x1520 modes with 10 bit readout - we get totally white frames.

It's only true at the new 1.8Gbit/s link frequency, so it's likely a duplicated register setting in the mode table vs programmatic configuration.

Don't know if it helps to narrow down the issue, but on looking at these two modes again, the metadata is showing a fixed 94 microseconds ExposureTime, and a fixed AnalogueGain of 1.0 while the white frames are being generated.

Separately, there looks to be an inconsistency in the /* Mode configs */ structure definition for imx477_mode supported_modes[].
In the 12Mpix cropped 16:9 mode section, the .crop.height is defined as 3040 but should be 2160 (??)

@6by9
Copy link
Contributor Author

6by9 commented Jul 5, 2024

Don't know if it helps to narrow down the issue, but on looking at these two modes again, the metadata is showing a fixed 94 microseconds ExposureTime, and a fixed AnalogueGain of 1.0 while the white frames are being generated.

Sadly not. It'll just be that the AE/AGC algorithms will have tried turning everything down as far as it can go because the statistics are seeing a saturated image.

Separately, there looks to be an inconsistency in the /* Mode configs */ structure definition for imx477_mode supported_modes[]. In the 12Mpix cropped 16:9 mode section, the .crop.height is defined as 3040 but should be 2160 (??)

I believe you're right, but in reality it is going to have limited effect.
I think the only thing it might mess up is lens shading tables due to trying to read 3040 lines starting at line ARRAY_TOP + 440, hence reading off the end.

Due to other time pressures I don't know when I'll get back to looking at this.

@schoolpost
Copy link

schoolpost commented Jul 18, 2024

Following up on the inquiry made here: https://forums.raspberrypi.com/viewtopic.php?p=2227965#p2227965

I'm attempting to get a 4000x3000 12-bit sensor mode to reach a minimum of 24fps. ( Ideally I'd love to use the full readout, but the next largest 4:3 pixel area is my target. )

I've tried to make the most sense out of which registers control crop based on the diff between the full readout and 2160 crop mode.
image
image

as such I've made some educated guesses on the what the controls do:

static const struct imx477_reg mode_4056x3040_regs[] = {
    {0x0344, 0x00},  
    {0x0345, 0x1c},  // X crop?
    {0x0346, 0x00},
    {0x0347, 0x14},  // Y crop? 20px oofset ( 20 + 20 = 40 )
    {0x0348, 0x0f},  
    {0x0349, 0xbb},  // X width  4027
    {0x034a, 0x0b},
    {0x034b, 0xcb},  // Y height? 3019
    {0x00e3, 0x00},
    {0x00e4, 0x00},  
    {0x00fc, 0x0a},
    {0x00fd, 0x0a},
    {0x00fe, 0x0a},
    {0x00ff, 0x0a},
    {0x0900, 0x00},
    {0x0901, 0x11},
    {0x3c01, 0x03},
    {0x3c02, 0xa2},
    {0x3f0d, 0x01},
    {0x5748, 0x07},
    {0x5749, 0xff},
    {0x574a, 0x00},
    {0x574b, 0x00},
    {0x7b75, 0x0a},
    {0x7b76, 0x0c},
    {0x7b77, 0x07},
    {0x7b78, 0x06},
    {0x7b79, 0x3c},
    {0x7b53, 0x01},
    {0x9369, 0x5a},
    {0x936b, 0x55},
    {0x936d, 0x28},
    {0x9304, 0x00},
    {0x9305, 0x00},
    {0xa2a9, 0x60},
    {0xa2b7, 0x00},
    {0x0401, 0x00},
    {0x0404, 0x00},
    {0x0405, 0x10},
    {0x0408, 0x00},
    {0x0409, 0x00},
    {0x040a, 0x00},
    {0x040b, 0x00},
    {0x040c, 0x0f},
    {0x040d, 0xa0}, // width 4000
    {0x040e, 0x0b},
    {0x040f, 0xb8},   // height 3000
    {0x034c, 0x0f},
    {0x034d, 0xa0},  // width 4000
    {0x034e, 0x0b},
    {0x034f, 0xb8},  // height 3000
    {0x0305, 0x04},
    {0x0306, 0x01},
    {0x0307, 0x5e},
    {0xe04c, 0x00},
    {0xe04d, 0x7f},
    {0xe04e, 0x00},
    {0xe04f, 0x1f},
    {0x3f56, 0x02},
    {0x3f57, 0xae},
};

and adjusting the mode and tightening up the line length just enough that I may reach just a touch over 24fps.

/* Mode configs */
static const struct imx477_mode supported_modes[] = {
    {
        /* 12MPix 10fps mode */
        .width = 4000,
        .height = 3000,
        .line_length_pix = { 23300, 20000 },
        .crop = {
            .left = IMX477_PIXEL_ARRAY_LEFT + 28,
            .top = IMX477_PIXEL_ARRAY_TOP + 20,
            .width = 4056,
            .height = 3040,
        },
        .framerate_default = 10,
        .reg_list = {
            .num_of_regs = ARRAY_SIZE(mode_4056x3040_regs),
            .regs = mode_4056x3040_regs,
        },
    },

It "almost" works, but clearly the crop is not functioning as it should as it is removing what appears to be the bottom 20-40 rows of pixels ( I haven't actually counted ), and after some hours of tinkering I cannot get it working properly.
image

Any further insights into getting something like this to work would be excellent!

@6by9
Copy link
Contributor Author

6by9 commented Jul 18, 2024

I haven't time to investigate this further at the moment.

As per #6208 (comment), the update is likely to be reducing the link rate to 1.5Gbit/s, so moving further away from your target. If I can easily make it generic, then I will. Simplest answer then is to go higher than 1.8Gbit/s and you will be able to get full res at 24fps.

I can't see anything obviously wrong with your register settings, however the datasheet does include a couple of restrictions on the start and end values for cropping. For no binning:

  • [XY]_ADD_STA must be a multiple of 4
  • X_ADD_END - X_ADD_STA + 1 must be a multiple of 4
  • Y_ADD_END - Y_ADD_STA + 1 must be a multiple of 8
    I don't see any of your settings violating those restrictions though.

0x0408 to 0x040f are the digital cropping registers (X offset, Y offset, width, height), so you may be able to leave the mode registers alone and digitally crop to get your desired result.

@schoolpost
Copy link

I haven't time to investigate this further at the moment.

As per #6208 (comment), the update is likely to be reducing the link rate to 1.5Gbit/s, so moving further away from your target. If I can easily make it generic, then I will. Simplest answer then is to go higher than 1.8Gbit/s and you will be able to get full res at 24fps.

I can't see anything obviously wrong with your register settings, however the datasheet does include a couple of restrictions on the start and end values for cropping. For no binning:

  • [XY]_ADD_STA must be a multiple of 4
  • X_ADD_END - X_ADD_STA + 1 must be a multiple of 4
  • Y_ADD_END - Y_ADD_STA + 1 must be a multiple of 8
    I don't see any of your settings violating those restrictions though.

0x0408 to 0x040f are the digital cropping registers (X offset, Y offset, width, height), so you may be able to leave the mode registers alone and digitally crop to get your desired result.

A moderate bump up to 1.9-2Gbits would be great... to get even a vague idea of registers requiring change and to the what values would be awesome! ; )

@KayhanB
Copy link

KayhanB commented Sep 11, 2024

Hi, I'm trying to pull this PR, but it fails with a 404 error. Is there another way to try this branch, and could you clarify why it fails?

@pelwell
Copy link
Contributor

pelwell commented Sep 11, 2024

The build artifacts are only retained for 90 days, and this is an old PR. I've rebased it now which has kicked off another build - try again in 45 minutes.

Registers 0x0342 / 0x0343 are set via IMX477_REG_LINE_LENGTH
as V4L2_CID_HBLANK, so shouldn't be in the register tables.

Registers 0x0340 / 0x0341 are set via IMX477_REG_FRAME_LENGTH
as V4L2_CID_VBLANK so shouldn't be in the register tables.

Registers 0x0112 and 0x0113 set the bit depth, so should be per
mode rather than in the common table and overridden for the 10bit
mode(s).

Signed-off-by: Dave Stevenson <[email protected]>
There are a fair number of registers duplicated in all the mode
tables, so move those into the common table.

Signed-off-by: Dave Stevenson <[email protected]>
The driver was using a struct v4l2_fract for the min frame
time to determine the range for V4L2_CID_VBLANK, and a
second to set the default VBLANK value for each mode.

However actually the sensor will accept any VBLANK value down
to 1 line, and using a struct v4l2_fract to hold the default
framerate (which is an integer in all cases) is rather overkill.

Drop the minimum frame time, and use a simple integer to set the
default. This actually increases the max frame rate in all modes
slightly.

Signed-off-by: Dave Stevenson <[email protected]>
The register set was always selecting continuous clock mode,
even though all our overlays were saying it should be non-continuous.

Read the configuration from fwnode and configure the sensor
accordingly.

Signed-off-by: Dave Stevenson <[email protected]>
Pi5 can support higher CSI2 link frequencies than Pi 0-4, and
hence higher framerates.

The simplest change is to change the DIV_IOP_PX divider from the
current value of 2 to 1 to double the frequency. This is slightly
outside the max rate nominally supported by RP1, but seems
reliable.

Signed-off-by: Dave Stevenson <[email protected]>
The driver can now support a link freq of 900MHz (1.8Gbit/s/lane)
for higher frame rates.

Add an override for "pi5" that changes this link frequency.

Signed-off-by: Dave Stevenson <[email protected]>
For 4k30 recording we want 16:9 output, so add a cropped mode
to achieve this.

Signed-off-by: Dave Stevenson <[email protected]>
FIXME: Dropping the default frame rate needs to be separated out.

Switching between 10 and 12 bit mode only requires a couple of
registers to change, and an associated change to the minimum
HBLANK that can be supported in the mode based on how long
it takes the CSI2 block to send each line of the image.

Add suitable switching between the 2 for all modes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants