On 06/12/2017 04:48 PM, Niklas Söderlund wrote:
Hi Hans,
Thanks for your comments.
On 2017-05-29 13:16:23 +0200, Hans Verkuil wrote:
On 05/24/2017 02:13 AM, Niklas Söderlund wrote:
From: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
A V4L2 driver for Renesas R-Car MIPI CSI-2 receiver. The driver
supports the rcar-vin driver on R-Car Gen3 SoCs where separate CSI-2
hardware blocks are connected between the video sources and the video
grabbers (VIN).
Driver is based on a prototype by Koji Matsuoka in the Renesas BSP.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
---
drivers/media/platform/rcar-vin/Kconfig | 12 +
drivers/media/platform/rcar-vin/Makefile | 1 +
drivers/media/platform/rcar-vin/rcar-csi2.c | 867 ++++++++++++++++++++++++++++
3 files changed, 880 insertions(+)
create mode 100644 drivers/media/platform/rcar-vin/rcar-csi2.c
+static int rcar_csi2_registered(struct v4l2_subdev *sd)
+{
+ struct rcar_csi2 *priv = container_of(sd, struct rcar_csi2, subdev);
+ struct v4l2_async_subdev **subdevs = NULL;
+ int ret;
+
+ subdevs = devm_kzalloc(priv->dev, sizeof(*subdevs), GFP_KERNEL);
+ if (subdevs == NULL)
+ return -ENOMEM;
+
+ subdevs[0] = &priv->remote.asd;
+
+ priv->notifier.num_subdevs = 1;
+ priv->notifier.subdevs = subdevs;
+ priv->notifier.bound = rcar_csi2_notify_bound;
+ priv->notifier.unbind = rcar_csi2_notify_unbind;
+ priv->notifier.complete = rcar_csi2_notify_complete;
+
+ ret = v4l2_async_subnotifier_register(&priv->subdev, &priv->notifier);
+ if (ret < 0) {
+ dev_err(priv->dev, "Notifier registration failed\n");
+ return ret;
+ }
+
+ return 0;
+}
Hmm, I'm trying to understand this, and I got one question. There are at least
two complete callbacks: rcar_csi2_notify_complete and the bridge driver's
complete callback. Am I right that the bridge driver's complete callback is
called as soon as this function exists (assuming this is the only subdev)?
Yes (at least for the async case).
In v4l2_async_test_notify() calls v4l2_device_register_subdev() which in
turns calls this registered callback. v4l2_async_test_notify() then go
on and calls the notifiers complete callback.
In my case I have (in the simplified case) AD7482 -> CSI-2 -> VIN. Where
VIN is the video device and CSI-2 is the subdevice of VIN while the
ADV7482 is a subdevice to the CSI-2. In that case the call graph would
be:
v4l2_async_test_notify() (From VIN on the CSI-2 subdev)
v4l2_device_register_subdev()
sd->internal_ops->registered(sd); (sd == CSI-2 subdev)
v4l2_async_subnotifier_register() (CSI-2 notifier for the ADV7482 subdev)
v4l2_async_test_notify() (From CSI-2 on the ADV7482) [1]
notifier->complete(notifier); (on the notifier from VIN)
So the bridge driver thinks it is complete when in reality this subdev may
be waiting on newly registered subdevs?
Yes if the ADV7482 subdevice are not already registered in [1] above the
VIN complete callback would be called before the complete callback have
been called on the notifier register from the CSI-2 registered callback.
Instead that would be called once the ADV7482 calls
v4l2_async_register_subdev().
If I am right, then my question is if that is what we want. If I am wrong,
then what did I miss?
I think that is what we want?
From the VIN point of view all the subdevices it registered in it's
notifier have been found and bound right so I think it's correct to call
the complete callback for that notifier at this point? If it really
cared about that all devices be present before it calls it complete
callback should it not also add all devices to its own notifier list?
But I do see your point that the VIN really have no way of telling if
all devices are present and we are ready to start to stream. This
however will be found out with a -EPIPE error if a stream is tried to be
started since the CSI-2 driver will fail to verify the pipeline since it
have no subdevice attached to its source pad. What do you think?
I think this is a bad idea. From the point of view of the application you
expect that once the device nodes appear they will also *work*. In this
case it might not work because one piece is still missing. So applications
would have to know that if they get -EPIPE, then if they wait for a few
seconds it might suddenly work because the last component was finally
loaded. That's IMHO not acceptable and will drive application developers
crazy.
In the example above the CSI-2 subdev can't tell VIN that it is complete
when it is still waiting for the ADV7482. Because it *isn't* complete.
It is also unexpected: if A depends on B and B depends on C and D, then
you expect that B won't tell A that is it ready unless C and D are both
loaded.
I was planning to merge this patch today: https://patchwork.linuxtv.org/patch/41834/
But I've decided to hold off on it for a bit in case changes are needed
to solve this particular issue. I think it is better to add that patch
as part of this driver's patch series.
Regards,
Hans
