Rpi/embedded data support/rpi 6.6.y

[jmondi]

This branch backports embedded data support to rpi-6.6.y

It includes framework changes to support embedded data and

• CFE from mainline
• Unicam from mainline
• Aligns imx219 with mainline
• Sensor patches from Naush to support embedded data

The branch compiles with rpi defconfig but fails for other drivers not part of defconfig. I'm working to backport the necessary changes there too.

The associated libcamera branch will be pushed soon.

[jmondi]

Refreshed the pull request, compared to v1:

• Test compiling with allmodconfig
• Compact backports at the beginning of the series
• Drop ov2740 multistream support
• Backport 2 BSP patches on mainline unicam
• Backport CFE from the patches merged in media-stage and not from the mailing list
• Fix imx477 multi-stream support reverting a BSP commit

[jmondi]

Added three patches from @jailuthra on top of Naush's one for sensors to add support for enable/disable_streams.

We kept them separate to ease review as they apply on top of Naush's ones. They can be squashed in later.

[jmondi]

Re-established the correct handling of analog binning mode for imx219 by applying https://github.com/raspberrypi/linux/pull/5498/commits/ef737ab00cdd895da3c0116398cf93139fd13e07

[naushir]

Starting to test this now together with https://github.com/raspberrypi/libcamera/pull/200 and all looks ok so far...

[jmondi]

Rebased on most recent rpi-6.6.y and apply the following change

@ -1025,7 +1025,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
     * embedded data stream.
     */
    ed_format = v4l2_subdev_state_get_format(state, IMX219_PAD_EDATA);
-   ed_format->code = imx219_format_edata(format->code);
+   ed_format->code = MEDIA_BUS_FMT_CCS_EMBEDDED;
    ed_format->width = format->width;
    ed_format->height = IMX219_EMBEDDED_DATA_HEIGHT;
    ed_format->field = V4L2_FIELD_NONE;
@ -1033,6 +1033,7 @@ static int imx219_set_pad_format(struct v4l2_subdev *sd,
    format = v4l2_subdev_state_get_format(state, IMX219_PAD_SOURCE,
                          IMX219_STREAM_EDATA);
    *format = *ed_format;
+   format->code = imx219_format_edata(format->code);

    if (fmt->which == V4L2_SUBDEV_FORMAT_ACTIVE) {
        u32 prev_hts = format->width + imx219->hblank->val;

To imx219, imx277, imx519 and imx708

[naushir]

Been testing on VC4 platforms, the one bit of functionality seems missing is allowing unpacked formats to be used. I think the following fixes it:

diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
index 1a6a0eacd49d..dce1047c3baa 100644
--- a/drivers/media/platform/broadcom/bcm2835-unicam.c
+++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
@@ -550,7 +550,8 @@ unicam_find_format_by_fourcc(u32 fourcc, u32 pad)
        }

        for (i = 0; i < num_formats; ++i) {
-               if (formats[i].fourcc == fourcc)
+               if (formats[i].fourcc == fourcc ||
+                   formats[i].unpacked_fourcc == fourcc)
                        return &formats[i];
        }

@6by9, our D/S unicam driver seems to do something with a check_variant flag in this bit of code that I can't quite understand. Do we want to add it back here?

[naushir]

Also PDAF does not seem to work correctly on Unicam + IMX708 - will need to check this next.

[6by9]

MEDIA_BUS_FMT_YUYV8_2X8 and MEDIA_BUS_FMT_YUYV8_1X16 both mapped to V4L2_PIX_FMT_YUYV (and the other component orderings).

Mainline try to say that 2X.. shouldn't be used for serial interfaces (only parallel where you can have multiple transfers per pixel). However that isn't true in all drivers, including adv7180. adv748x afe got fixed up recently.

I have fixed adv7180 up downstream in https://github.com/raspberrypi/linux/commit/4f9944c95eb69ef9ebbc8fe6eceaf1de43ab122d, and that's the only device we really care about that needed this workaround. (Yes, another patch I ought to send upstream)

[naushir]

Also PDAF does not seem to work correctly on Unicam + IMX708 - will need to check this next.

For some reason, the first frame has invalid PDAF/HDR metadata when running with Unicam, subsequent frames are fine. CFE/Pi 5 does not seem to show this problem. @njhollinghurst any thoughts as to why this may be? The following test in the helper fails:

if (bpp < 10 || bpp > 14 || len < 194 * step || ptr[0] != 0 || ptr[1] >= 0x40) {

because ptr[0] != 0.

[njhollinghurst]

Of course it means the PDAF data packet was corrupt (or the packet at that position was not PDAF). It's a pretty weak validation -- only 2 bytes are checked -- and since the driver doesn't say how many bytes were received, we can't tell if we're checking current or stale buffer contents. EDIT: The nonzero value does at least suggest we received something...

Is it possible that this always happens (in which case the warning should maybe be suppressed), but in other cases we drop the entire first metadata buffer so the check is never performed?

[naushir]

In the old (i.e. existing) codebase, the first frame coming out of the sensor/Unicam goes through to the helper and gets parsed without errors. I'll trace this again with the new code.

[jmondi]

MEDIA_BUS_FMT_YUYV8_2X8 and MEDIA_BUS_FMT_YUYV8_1X16 both mapped to V4L2_PIX_FMT_YUYV (and the other component orderings).

Mainline try to say that 2X.. shouldn't be used for serial interfaces (only parallel where you can have multiple transfers per pixel). However that isn't true in all drivers, including adv7180. adv748x afe got fixed up recently.

I have fixed adv7180 up downstream in 4f9944c, and that's the only device we really care about that needed this workaround. (Yes, another patch I ought to send upstream)

Hi Dave do you think you will need to keep the support for the 2X8 formats once [4f9944c] is upstreamed ?

[6by9]

Hi Dave do you think you will need to keep the support for the 2X8 formats once [4f9944c] is upstreamed ?

No, it's officially the wrong thing to do, and doesn't impact any of the source drivers that we care about. If someone tries adding support for a driver that "does the wrong thing", they get told to fix up the source driver.

At the time I originally wrote the Unicam driver I don't think 2X formats had been officially been stated as not applicable for CSI, hence working around the source driver quirks.

[naushir]

Wow, this was hard to track down, but I think I've fixed the Unicam PDAF/HDR metadata problem. In fact, I had fixed it 2 years ago with 59df4fce89111dd1cc770cb8d725a85d4467ac0c. With this change applied things seem to behave consistently across the existing codebase and this new set of changes.

But I probably ought to check that all our other downstream fixes have also made it into the driver.

[naushir]

Sadly there are a couple more ISR fixes missing from the upstream driver. No way to cherry pick single commits, I've got a commit that fixes everything here: https://github.com/naushir/linux/commit/26b3a0932297e9cf37091c699f3019f28623bbca

[jmondi]

Sadly there are a couple more ISR fixes missing from the upstream driver. No way to cherry pick single commits, I've got a commit that fixes everything here: naushir@26b3a09

Thank you for your hard work in chasing down issues with the mainline unicam driver.

Would you like me to check if changes from https://github.com/naushir/linux/commit/26b3a0932297e9cf37091c699f3019f28623bbca can be broken down to single commits and applied to the mainline version ?

[naushir]

Would you like me to check if changes from naushir@26b3a09 can be broken down to single commits and applied to the mainline version ?

No need, I'm in the process of doing that right now. Basically 3 commits - 1) Dummy buffer 0 size, 2) format change 3) ISR robustness improvements. I'll post the patches shortly.

[naushir]

I think the top 4 commit from https://github.com/naushir/linux/commits/streams/ should be added to this and it looks to be passing all my tests on VC4.

[jmondi]

Rebased on latest v6.6.y with @naushir patches on top.

Tested on rpi4 with imx708

[0:21:30.109693327] [1825] DEBUG V4L2 v4l2_videodevice.cpp:1731 /dev/video1[19:cap]: Dequeuing buffer 0
[0:21:30.109772196] [1825] DEBUG RPI vc4.cpp:894 Stream Unicam Embedded buffer dequeue, buffer id 1, timestamp: 1290097911000
[0:21:30.109993951] [1825] DEBUG RPI vc4.cpp:1078 Signalling prepareIsp: Bayer buffer id: 1
[0:21:30.110038061] [1825] DEBUG RPI vc4.cpp:1093 Signalling prepareIsp: Embedded buffer id: 1
[0:21:30.110208650] [1825] DEBUG Event event_dispatcher_poll.cpp:215 next timer 0xf38278e8 expires in 0.998666639
[0:21:30.110830674] [1828] DEBUG IPARPI ipa_base.cpp:1299 Metadata - Exposure: 994.58us Frame length: 3619 Line length: 9.21us Gain: 1.12281 Lens: 1

Sorry for not noticing before, I didn't have an imx708 until a few days ago

[njhollinghurst]

FWIW, it was probably not imx708 specific -- it's just that the imx708 PDAF/HDR metadata parser prints these warnings. Other camera helpers might silently fail (or succeed, if the required metadata came before the data loss).