libcamera: all connections unavailable until all sensor drivers loaded

[kitakar5525]

~#### FIXME: connection between IPU3 and sensors not working if there is more than one connection~

~Somehow notworking if there are multiple connections.~ ~So, I reverted CIO2-CAMF and CIO2-CAMR connections for CIO2-CAM3 connection.~

It turned out that enabling connections for both CIO2-CAMR and CIO2-CAM3 at the same time breaks all the connections between CIO2 and sensors. This is maybe because CAMR and CAM3 is on the same I2C bus (I2C3).

This is the case for SP4,5,6,7 and SB1,2,3.

For now, I disable CIO2-CAMR connection: https://github.com/kitakar5525/surface-ipu3-cameras/commit/270a1c3ae32e45f3b242cf20974623e67e7dca61

[kitakar5525]

I've been misunderstanding. Rather, the connections don't break but libcamera somehow won't recognize all the connections until all the sensor drivers are loaded.

Confirmed connections don't break by adding debug print to ipu3-cio2:

0001-media-ipu3-cio2-cio2_parse_firmware-add-debug-print.patch

```diff From d3f71aceb2d5692f5bf48e5eeeb9d7e18775ca38 Mon Sep 17 00:00:00 2001 From: Tsuchiya Yuto Date: Thu, 9 Jul 2020 17:03:29 +0900 Subject: [PATCH 1/3] media: ipu3-cio2: cio2_parse_firmware(): add debug print Signed-off-by: Tsuchiya Yuto --- drivers/media/pci/intel/ipu3/ipu3-cio2.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/media/pci/intel/ipu3/ipu3-cio2.c b/drivers/media/pci/intel/ipu3/ipu3-cio2.c index 92f5eadf2c99f..7098cfad9417b 100644 --- a/drivers/media/pci/intel/ipu3/ipu3-cio2.c +++ b/drivers/media/pci/intel/ipu3/ipu3-cio2.c @@ -1491,12 +1491,21 @@ static int cio2_parse_firmware(struct cio2_device *cio2) dev_fwnode(&cio2->pci_dev->dev), i, 0, FWNODE_GRAPH_ENDPOINT_NEXT); - if (!ep) + if (!ep) { + dev_info(&cio2->pci_dev->dev, + "%s(), endpoint not available for CIO2 port %d\n", + __func__, i); continue; + } ret = v4l2_fwnode_endpoint_parse(ep, &vep); - if (ret) + if (ret) { + dev_info(&cio2->pci_dev->dev, + "%s(), v4l2_fwnode_endpoint_parse() failed for CIO2 port %d " + ": (ret: %d)\n", + __func__, i, ret); goto err_parse; + } s_asd = kzalloc(sizeof(*s_asd), GFP_KERNEL); if (!s_asd) { @@ -1509,11 +1518,20 @@ static int cio2_parse_firmware(struct cio2_device *cio2) ret = v4l2_async_notifier_add_fwnode_remote_subdev( &cio2->notifier, ep, &s_asd->asd); - if (ret) + if (ret) { + dev_info(&cio2->pci_dev->dev, + "%s(), v4l2_async_notifier_add_fwnode_remote_subdev() failed " + "for CIO2 port %d : (ret: %d)\n", + __func__, i, ret); goto err_parse; + } fwnode_handle_put(ep); + dev_info(&cio2->pci_dev->dev, + "%s(), CIO2 port %d available: csi2.port: %d csi2.lanes: %d\n", + __func__, i, s_asd->csi2.port, s_asd->csi2.lanes); + continue; err_parse: -- 2.27.0 ```

$ dmesg -x
[...]
[    3.826600] kernel: ipu3-cio2 0000:00:14.3: cio2_parse_firmware(), CIO2 port 0 available: csi2.port: 0 csi2.lanes: 4
[    3.826613] kernel: ipu3-cio2 0000:00:14.3: cio2_parse_firmware(), CIO2 port 1 available: csi2.port: 1 csi2.lanes: 2
[    3.826624] kernel: ipu3-cio2 0000:00:14.3: cio2_parse_firmware(), CIO2 port 2 available: csi2.port: 2 csi2.lanes: 1
[    3.826632] kernel: ipu3-cio2 0000:00:14.3: cio2_parse_firmware(), endpoint not available for CIO2 port 3
[...]

[kbingham]

This might be more to do with V4L2 than libcamera. V4L2 expects the whole media graph to be completed before allowing things to work.

[djrscally]

Actually this is the fault of the ipu3-cio2 driver and the async subdev registration. Long story short is the ipu3-cio2 parses firmware and adds each remote subdev it finds via v4l2_async_notifier_add_fwnode_remote_subdev(). The notifier has two callbacks that can handle a device being added; .bound() which is called when a sensor probes, and .complete() which is called when ALL of the remote subdevs added by v4l2_async_notifier_add_fwnode_remote_subdev() have probed.

A lot of fundamental setup (create and enable links, register subdev nodes) is delayed by ipu3-cio2 until the .complete() callback:

/* .complete() is called after all subdevices have been located */
static int cio2_notifier_complete(struct v4l2_async_notifier *notifier)
{
    struct cio2_device *cio2 = container_of(notifier, struct cio2_device,
                        notifier);
    struct sensor_async_subdev *s_asd;
    struct v4l2_async_subdev *asd;
    struct cio2_queue *q;
    unsigned int pad;
    int ret;

    list_for_each_entry(asd, &cio2->notifier.asd_list, asd_list) {
        s_asd = container_of(asd, struct sensor_async_subdev, asd);
        q = &cio2->queue[s_asd->csi2.port];

        for (pad = 0; pad < q->sensor->entity.num_pads; pad++)
            if (q->sensor->entity.pads[pad].flags &
                        MEDIA_PAD_FL_SOURCE)
                break;

        if (pad == q->sensor->entity.num_pads) {
            dev_err(&cio2->pci_dev->dev,
                "failed to find src pad for %s\n",
                q->sensor->name);
            return -ENXIO;
        }

        ret = media_create_pad_link(
                &q->sensor->entity, pad,
                &q->subdev.entity, CIO2_PAD_SINK,
                0);
        if (ret) {
            dev_err(&cio2->pci_dev->dev,
                "failed to create link for %s\n",
                q->sensor->name);
            return ret;
        }
    }

    return v4l2_device_register_subdev_nodes(&cio2->v4l2_dev);
}

So, we could fix this by moving most of that to .bound() if we thought it was a big enough problem.

[kbingham]

@djrscally Probably worth running that by Hans on #v4l / linux-media list. There are some odd rules about a media graph being complete entirely before it is supported. Which (as we see here) can be really annoying if a fully functional device is prevented from operating because of a separate (non blocking) device having not been probed.

In my opinion, we should support the cameras which are successfully probed of course ;-)