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

PiSP support for RAW14 MIPI Sensor? #172

Open
schoolpost opened this issue Sep 4, 2024 · 23 comments
Open

PiSP support for RAW14 MIPI Sensor? #172

schoolpost opened this issue Sep 4, 2024 · 23 comments

Comments

@schoolpost
Copy link

Having some issues with getting a RAW14 capable MIPI sensor working in Libcamera. The driver is currently under development such that it is possible there are issues on that end, but also looking at some of the PiSP docs, I don't see any explicit mention of support for this format? which would really suck :(

This is what get's logged when attempting to launch a rpicam-apps instance that calls for RAW14 mode:

CMD: rpicam-hello -t 0 --viewfinder-mode 3792:2840:14:U --width 3704 --height 2778 --denoise "off" --shutter 20833 --analoggain 1 --framerate 10 --awb="auto"

[2:51:25.384736617] [2915]  INFO Camera camera_manager.cpp:316 libcamera v0.3.1+49-48fe316f-dirty (2024-09-01T23:23:35MDT)
[2:51:25.385850240] [2916]  INFO RPI pisp.cpp:695 libpisp version v1.0.7 28196ed6edcf 01-09-2024 (19:58:40)
[2:51:25.387183027] [2916]  WARN CameraSensorProperties camera_sensor_properties.cpp:295 No static properties available for 'imx294'
[2:51:25.387212935] [2916]  WARN CameraSensorProperties camera_sensor_properties.cpp:297 Please consider updating the camera sensor properties database
[2:51:25.391926016] [2916]  INFO RPI pisp.cpp:1154 Registered camera /base/axi/pcie@120000/rp1/i2c@88000/imx294@1a to CFE device /dev/media0 and ISP device /dev/media2 using PiSP variant BCM2712_D0
Made DRM preview window
[2:51:25.496723767] [2915] ERROR RPI pipeline_base.cpp:228 Invalid sensor configuration: bitDepth/size mismatch
[2:51:25.496768637] [2915] ERROR RPI pipeline_base.cpp:228 Invalid sensor configuration: bitDepth/size mismatch
[2:51:25.496781730] [2915] ERROR Camera camera.cpp:1179 Can't configure camera with invalid configuration
Mode selection for 3792:2840:14:U(10)
    SRGGB12_CSI2P,3872x2180/61.4288 - Score: 2505.35
    SRGGB12_CSI2P,4144x2184/57.7601 - Score: 2610.83
    SRGGB12_CSI2P,4176x2184/54.3685 - Score: 2624.33
    SRGGB14_CSI2P,3792x2840/54.3685 - Score: 1000
[2:51:25.497188560] [2915] ERROR RPI pipeline_base.cpp:228 Invalid sensor configuration: bitDepth/size mismatch
ERROR: *** failed to valid stream configurations ***
@naushir
Copy link
Collaborator

naushir commented Sep 5, 2024

Sadly there is an issue with RAW14 (just like RAW16 had issues). So we need to add some software RAW14->16 conversion in the pipeline handler. I don't have an eta on this feature yet though.

On a side note, this is not actually the problem you see above. It seems like something is not accurately reporting the format or size in 14-bit more. Perhaps the device driver, or the CFE driver, it's hard to tell. Does dmesg give you any clues?

@schoolpost
Copy link
Author

Sadly there is an issue with RAW14 (just like RAW16 had issues). So we need to add some software RAW14->16 conversion in the pipeline handler. I don't have an eta on this feature yet though.

On a side note, this is not actually the problem you see above. It seems like something is not accurately reporting the format or size in 14-bit more. Perhaps the device driver, or the CFE driver, it's hard to tell. Does dmesg give you any clues?

No unfortunately dmesg doesn't reveal much:

[    8.242902] rp1-cfe 1f00110000.csi: Using sensor imx294 6-001a for capture
[    8.243008] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-csi2_ch0] node id 0 successfully as /dev/video0
[    8.243052] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-embedded] node id 1 successfully as /dev/video1
[    8.243218] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-csi2_ch2] node id 2 successfully as /dev/video2
[    8.244133] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-csi2_ch3] node id 3 successfully as /dev/video3
[    8.244440] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-fe_image0] node id 4 successfully as /dev/video4
[    8.245729] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-fe_image1] node id 5 successfully as /dev/video5
[    8.247358] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-fe_stats] node id 6 successfully as /dev/video6
[    8.248605] rp1-cfe 1f00110000.csi: Registered [rp1-cfe-fe_config] node id 7 successfully as /dev/video7
[    8.470020] Adding 204784k swap on /var/swap.  Priority:-2 extents:8 across:843776k SS
[    9.199841] macb 1f00100000.ethernet eth0: PHY [1f00100000.ethernet-ffffffff:01] driver [Broadcom BCM54213PE] (irq=POLL)
[    9.199851] macb 1f00100000.ethernet eth0: configuring for phy/rgmii-id link mode
[    9.203297] pps pps0: new PPS source ptp0
[    9.203352] macb 1f00100000.ethernet: gem-ptp-timer ptp clock registered.
[    9.234128] brcmfmac: brcmf_cfg80211_set_power_mgmt: power save enabled
[    9.739277] cinepi-raw[853]: memfd_create() called without MFD_EXEC or MFD_NOEXEC_SEAL set
[    9.946048] source of link 'rp1-cfe-fe_config':0->'pisp-fe':1 is not a V4L2 sub-device, driver bug!
[    9.946073] v4l2_get_link_freq: Link frequency estimated using pixel rate: result might be inaccurate
[    9.946074] v4l2_get_link_freq: Consider implementing support for V4L2_CID_LINK_FREQ in the transmitter driver
[    9.946076] rp1-cfe 1f00110000.csi: Using a link rate of 1599 Mbps
[    9.946127] rp1-cfe 1f00110000.csi: DPHY: Datarate 1599 Mbps out of range
[   13.290091] macb 1f00100000.ethernet eth0: Link is Up - 1Gbps/Full - flow control off`

The driver is here: https://github.com/will127534/imx294-v4l2-driver

The default driver implementation only contains 12-bit modes, but I have gone ahead and added a couple 10-bit modes successfully, but libcamera errors are only present with 14-bit modes.

I'm ready to test / patch things as required to get this mode working. Is there a rough area I can be pointed to as far as fixing this?

@naushir
Copy link
Collaborator

naushir commented Sep 10, 2024

I think your error is because rpicam-apps does not know about 14-bit modes. Try adding them to the following table:
https://github.com/raspberrypi/rpicam-apps/blob/d7a1a13b041ef2842cd56d7e395b8c9a0ffc3bf5/core/options.cpp#L35

However, as mentioned earlier you will not get valid pixel data output as we need to have some software fixups.

@schoolpost
Copy link
Author

schoolpost commented Sep 10, 2024

Still running into an identical error:

[0:04:01.875500698] [3456]  INFO RPI pisp.cpp:695 libpisp version v1.0.7 28196ed6edcf 01-09-2024 (19:58:40)
[0:04:01.876849393] [3456]  WARN CameraSensorProperties camera_sensor_properties.cpp:295 No static properties available for 'imx294'
[0:04:01.876879656] [3456]  WARN CameraSensorProperties camera_sensor_properties.cpp:297 Please consider updating the camera sensor properties database
[0:04:01.882751282] [3456]  INFO RPI pisp.cpp:1154 Registered camera /base/axi/pcie@120000/rp1/i2c@88000/imx294@1a to CFE device /dev/media2 and ISP device /dev/media0 using PiSP variant BCM2712_D0
Overriding H.264 level 4.2
[2024-09-10 13:10:38.936] [cinepi_controller] [info] CinePIController Started!
Preview window unavailable
[0:04:01.963788789] [3455] ERROR RPI pipeline_base.cpp:231 Invalid sensor configuration: bitDepth/size mismatch
[0:04:01.963916821] [3455] ERROR RPI pipeline_base.cpp:231 Invalid sensor configuration: bitDepth/size mismatch
[0:04:01.963938361] [3455] ERROR Camera camera.cpp:1179 Can't configure camera with invalid configuration
[2024-09-10 13:10:39.000] [dng_encoder] [info] DngEncoder started!
Mode selection for 3792:2840:14:U(24)
    SRGGB10_CSI2P,2088x1096/241.255 - Score: 9109.71
    SRGGB10_CSI2P,1896x1404/190.223 - Score: 8669.71
    SRGGB12_CSI2P,3872x2180/61.4288 - Score: 2505.35
    SRGGB12_CSI2P,4144x2184/57.7601 - Score: 2610.83
    SRGGB12_CSI2P,4176x2184/54.3685 - Score: 2624.33
    SRGGB12_CSI2P,3792x2840/48.695 - Score: 1000
    SRGGB14_CSI2P,3792x2840/48.695 - Score: 1000
[0:04:01.965317911] [3455] ERROR RPI pipeline_base.cpp:231 Invalid sensor configuration: bitDepth/size mismatch```

and

Available cameras
-----------------
0 : imx294 [3840x2160 14-bit RGGB] (/base/axi/pcie@120000/rp1/i2c@88000/imx294@1a)
    Modes: 'SRGGB10_CSI2P' : 2088x1096 [241.25 fps - (-4, -6)/4096x2160 crop]
                             1896x1404 [190.22 fps - (0, -2)/3704x2778 crop]
           'SRGGB12_CSI2P' : 3872x2180 [61.43 fps - (-20, -6)/3840x2160 crop]
                             4144x2184 [57.76 fps - (-4, -6)/4096x2160 crop]
                             4176x2184 [54.37 fps - (-4, -6)/4096x2160 crop]
                             3792x2840 [48.69 fps - (0, -2)/3704x2778 crop]
           'SRGGB14_CSI2P' : 3792x2840 [48.69 fps - (0, -2)/3704x2778 crop]

There must be more on the libcamera end to do with this? notice this line 'SRGGB14_CSI2P' : 3792x2840 [48.69 fps - (0, -2)/3704x2778 crop] contains the same computed frame rate as the 12-bit mode, but this is wrong as described in the datasheet for the sensor; this A/D resolution comes at a lower maximum frame rate.

@naushir
Copy link
Collaborator

naushir commented Sep 11, 2024

There must be more on the libcamera end to do with this? notice this line 'SRGGB14_CSI2P' : 3792x2840 [48.69 fps - (0, -2)/3704x2778 crop] contains the same computed frame rate as the 12-bit mode, but this is wrong as described in the datasheet for the sensor; this A/D resolution comes at a lower maximum frame rate.

Can you share the change you made to rpicam-apps please?

Regarding the framerate number, libcamera computes the framerate with the horizontal/vertical blanking limits that are reported by the kernel device driver. Perhaps they are not setup correctly for the 14-bit mode?

@schoolpost
Copy link
Author

schoolpost commented Sep 11, 2024

Can you share the change you made to rpicam-apps please?

diff --git a/core/options.cpp b/core/options.cpp
index da49e58..bb9931c 100644
--- a/core/options.cpp
+++ b/core/options.cpp
@@ -43,6 +43,10 @@ static const std::map<libcamera::PixelFormat, unsigned int> bayer_formats =
 	{ libcamera::formats::SGRBG12_CSI2P, 12 },
 	{ libcamera::formats::SBGGR12_CSI2P, 12 },
 	{ libcamera::formats::SGBRG12_CSI2P, 12 },
+	{ libcamera::formats::SRGGB14_CSI2P, 14 },
+	{ libcamera::formats::SGRBG14_CSI2P, 14 },
+	{ libcamera::formats::SBGGR14_CSI2P, 14 },
+	{ libcamera::formats::SGBRG14_CSI2P, 14 },
 	{ libcamera::formats::SRGGB16,       16 },
 	{ libcamera::formats::SGRBG16,       16 },
 	{ libcamera::formats::SBGGR16,       16 },
diff --git a/core/rpicam_app.cpp b/core/rpicam_app.cpp
index 7c1174f..46962ad 100644
--- a/core/rpicam_app.cpp
+++ b/core/rpicam_app.cpp
@@ -39,6 +39,10 @@ static libcamera::PixelFormat mode_to_pixel_format(Mode const &mode)
 		{ Mode(0, 0, 10, true), libcamera::formats::SBGGR10_CSI2P },
 		{ Mode(0, 0, 12, false), libcamera::formats::SBGGR12 },
 		{ Mode(0, 0, 12, true), libcamera::formats::SBGGR12_CSI2P },
+		{ Mode(0, 0, 14, false), libcamera::formats::SBGGR14 },
+		{ Mode(0, 0, 14, true), libcamera::formats::SBGGR14_CSI2P },
+		{ Mode(0, 0, 16, false), libcamera::formats::SBGGR16 },
+		{ Mode(0, 0, 16, true), libcamera::formats::SBGGR16 },
 	};
 
 	auto it = std::find_if(table.begin(), table.end(), [&mode] (auto &m) { return mode.bit_depth == m.first.bit_depth && mode.packed == m.first.packed; });

Regarding the framerate number, libcamera computes the framerate with the horizontal/vertical blanking limits that are reported by the kernel device driver. Perhaps they are not setup correctly for the 14-bit mode?

These are the parameters for the readout mode in 12-bit:

    {
        /* 3740 x 2778 readout mode 0 */
        .width = 3792,
        .height = 2840,
        .min_HMAX = 1024,
        .min_VMAX = 1444,
        .default_HMAX = 1875,
        .default_VMAX = 1600, 
        .VMAX_scale = 2,
        .min_SHR = 5,
        .integration_offset = 551,
        .crop = {
            .left = 40,
            .top = 24,
            .width = 3704,
            .height = 2778,
        },
        .reg_list = {
            .num_of_regs = ARRAY_SIZE(mode_00_regs),
            .regs = mode_00_regs,
        },
    },

and for 14-bit:

        {
        /* 3740 x 2778 readout mode 0 */
        .width = 3792,
        .height = 2840,
        .min_HMAX = 1578,
        .min_VMAX = 1444,
        .default_HMAX = 1650,
        .default_VMAX = 1456, 
        .VMAX_scale = 2,
        .min_SHR = 5,
        .integration_offset = 551,
        .crop = {
            .left = 40,
            .top = 24,
            .width = 3704,
            .height = 2778,
        },
        .reg_list = {
            .num_of_regs = ARRAY_SIZE(mode_00_14bit_regs),
            .regs = mode_00_14bit_regs,
        },
    },

so there are differences in VMAX and HMAX that should result in different max frame rate?

@naushir
Copy link
Collaborator

naushir commented Sep 11, 2024

Just as an aside - it might be possible to workaround the hardware limitation where we don't need to software post-process the RAW-14 data. Do you know if this sensor can set a custom CSI2 datatype in the packet header? Normally for 14-bit data, the value is 0x2D. If you can change this to 0x2E, 14-bit should work (above software problems aside), including statistics gathering and everything.

@schoolpost
Copy link
Author

schoolpost commented Sep 11, 2024

Just as an aside - it might be possible to workaround the hardware limitation where we don't need to software post-process the RAW-14 data. Do you know if this sensor can set a custom CSI2 datatype in the packet header? Normally for 14-bit data, the value is 0x2D. If you can change this to 0x2E, 14-bit should work (above software problems aside), including statistics gathering and everything.

I assume 0x2E would be to output as RAW16? I don't see that as an option in the datasheet:
image

@naushir
Copy link
Collaborator

naushir commented Sep 11, 2024

That's correct, this is a bug in our CSI2-RX that it treats 16-bit data (datatype 0x2E) as 14-bit. Some sensors have the ability to set custom data-type values. So if you can set the sensor up for 14-bit output, but set the datatype as 16-bit (0x2E), it will workaround the bug in our CSI2-RX.

@schoolpost
Copy link
Author

That's correct, this is a bug in our CSI2-RX that it treats 16-bit data (datatype 0x2E) as 14-bit. Some sensors have the ability to set custom data-type values. So if you can set the sensor up for 14-bit output, but set the datatype as 16-bit (0x2E), it will workaround the bug in our CSI2-RX.

If that options exists, it's not present in any of the documentation I have access to...

CSI2-RX bug is hardware or can be patched in firmware?

@naushir
Copy link
Collaborator

naushir commented Sep 12, 2024

CSI2-RX bug is hardware or can be patched in firmware?

No this cannot be patched in the firmware. We must postprocess the RAW data to generate a sensible output.

@schoolpost
Copy link
Author

CSI2-RX bug is hardware or can be patched in firmware?

No this cannot be patched in the firmware. We must postprocess the RAW data to generate a sensible output.

Assuming a similar approach can be taken as the endianness fix for the 16-bit RAW from the IMX585.

I'm a little stumped as to what measures need to be taken; given that Libcamera hard crashes with the error I have shown above. With the 16-bit RAW endianness issue, although the pixel data was improper, frames were passing through just as normal, not the case with 14-bit RAW.

I've taken a look here at the error itself:

if (bayer.bitDepth != sensorConfig->bitDepth ||

The only further info I could dig out is that the mismatch is that one of them is reported as 14 and the other as 16 ( assuming this is the exact behaviour you are describing in the CSI2-RX bug )

is the CSI2-RX source available? can I modify this and test?

@naushir
Copy link
Collaborator

naushir commented Sep 13, 2024

No, what you are hitting is a purely software issue.

Is sensorConfig->bitDepth set to 14 or 16? This is that rpicam-apps passes into the pipeline handler, and should be 14. You might also want to look at what values bayer and sensorFormat_.mbus_code are.

@naushir
Copy link
Collaborator

naushir commented Sep 13, 2024

The CSI2 RX driver source can be found at https://github.com/raspberrypi/linux/blob/rpi-6.6.y/drivers/media/platform/raspberrypi/rp1_cfe/cfe.c, but I doubt there is a problem in that.

@schoolpost
Copy link
Author

No, what you are hitting is a purely software issue.

Is sensorConfig->bitDepth set to 14 or 16? This is that rpicam-apps passes into the pipeline handler, and should be 14. You might also want to look at what values bayer and sensorFormat_.mbus_code are.

Solved a piece to this issue, here in rpicam_app.hpp; missing a 14-bit option: https://github.com/raspberrypi/rpicam-apps/blob/d7a1a13b041ef2842cd56d7e395b8c9a0ffc3bf5/core/rpicam_app.hpp#L90

		unsigned int depth() const
		{
			// This is a really ugly way of getting the bit depth of the format.
			// But apart from duplicating the massive bayer format table, there is
			// no other way to determine this.
			std::string fmt = format.toString();
			unsigned int mode_depth = fmt.find("8") != std::string::npos ? 8 :
									  fmt.find("10") != std::string::npos ? 10 :
									  fmt.find("12") != std::string::npos ? 12 :
									  fmt.find("14") != std::string::npos ? 14 : 16;
			return mode_depth;
		}

Now, as expected; the output image is a mess of noisy / malformed pixel data. But I can't tell if it's simply a matter of a bitshift, incorrect endianness or that the RAW is not properly being unpacked? or any number of combination of those

@schoolpost
Copy link
Author

schoolpost commented Sep 13, 2024

I've gone into Vooya to view the RAW pixel data, this is what I get:

image
raw14_srggb16_vooya

Find attached the .RAW file:
output_image.zip

My suspicion is RAW14 is not getting properly unpacked ( hence the large black region near the bottom of the frame )

@naushir
Copy link
Collaborator

naushir commented Sep 14, 2024

Yes, this now looks like an unpacking issue with the hardware that we expect. When I get a chance, I will write a SW unpacker that should give you a correct looking image. But like the 16-bit case, that statistics will not be available.

Would you be able to create a PR for the rpicam-apps changes that you made for 14-bit support?

@schoolpost
Copy link
Author

schoolpost commented Sep 14, 2024

Yes, this now looks like an unpacking issue with the hardware that we expect. When I get a chance, I will write a SW unpacker that should give you a correct looking image. But like the 16-bit case, that statistics will not be available.

Would you be able to create a PR for the rpicam-apps changes that you made for 14-bit support?

That will be a acceptable compromise just as with the 16-bit format. I hope to see the changes get pushed into future revisions of the hardware.

PR Opened.

@schoolpost
Copy link
Author

Yes, this now looks like an unpacking issue with the hardware that we expect. When I get a chance, I will write a SW unpacker that should give you a correct looking image. But like the 16-bit case, that statistics will not be available.

Would you be able to create a PR for the rpicam-apps changes that you made for 14-bit support?

Let me know if there is approximate timeline for this, I am very eager to test. Thanks again!

@schoolpost
Copy link
Author

Hi @naushir just following up to see if there is any progress on this? I realize this isn't a trivial task, I attempted an implementation myself... but noticed there are a couple of hurdles to deal with with; one to do with the padding added by CFE which makes it less straight forward than just iterating over the data sequentially and unpacking. Also likely needing to cache the at least 2 rows worth of data...

@naushir
Copy link
Collaborator

naushir commented Sep 27, 2024

Unfortunately it's going to be a while before I get to looking at this, sorry :(

@schoolpost
Copy link
Author

Any chance of a revisit anytime soon?

@naushir
Copy link
Collaborator

naushir commented Nov 5, 2024

Not yet I'm afraid, there's just too much more going on right now that I have to work on.

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

No branches or pull requests

2 participants