Re: [PATCH] media: intel-ipu3: cio2: register the mdev on v4l2 async notifier complete

[Javier Martinez Canillas]

Hi Sakari,  Tian Shu,

On 09/04/2018 08:46 AM, Sakari Ailus wrote:
> Hi Javier, Tian Shu,
> 
> On Tue, Sep 04, 2018 at 05:01:56AM +0000, Qiu, Tian Shu wrote:
>> Hi,
>>
>> Raise my point.
>> The case here is that we have multiple sensors connected to CIO2. The sensors work independently. So failure on one sensor should not block the function of the other.
>> That is, we should not rely on that all sensors are ready before allowing user to operate on the ready cameras.
>> Sometimes due to hardware issues or incompleteness, we did met the case that one sensor is not probing properly. And in this case, the current implementation blocks us using the working one.

This is a valid concern, but unfortunately the media graph is created in a quite
static way currently: the port and enpoints are parsed in the hardware topology,
the driver waits for the subdevices to be registered and bound, and is notified
by the .complete callback when there aren't more pending devices to be registered.

With the current infrastructure, I think we have to wait to register the media
device node until all devices have been bound, even if that would mean a single
camera driver not probing will cause the media graph to not be available.

Probably the correct approach would be to allow the media graph to dynamically
change and notify the user when new nodes are added or deleted (currently we
prevent to remove modules for the devices bound to the media device).

>> What I can think now to solve this are:
>> 1. Register multiple media devices. One for each sensor path. This will increase media device count.
>> 2. Use .bound callback to create the link and register the subdev node for each sensor. Leave .complete empty.
>>      Not sure if this breaks the rule of media framework. And also have not found an API to register one single subdev node.
> 
> I'd prefer to keep the driver as-is.
> 
> Even if the media device is only created once all the sub-devices are
> around, the devices are still created one by one so there's no way to
> prevent the user space seeing a partially registered media device complex.
>

Yes, but at least the media device node won't be available.

> In general that doesn't happen as the sensors are typically registered
> early during system boot.
>

It's true if the drivers are built-in, but they can be built as modules.

The goal with this patch was to prevent having a media graph that's not
useful, for example I've a media graph with no sensors media entities
(because there are no drivers supporting the sensors on my machine).

I prefer not having a media node in that case, that way users can know
that something went wrong / some support is missing.

> Javier is right in asking a way for the user to know whether everything is
> fully initialised. That should be added but I don't think it is in any way
> specific to the cio2 driver.
>

I was thinking about using the presence of the media device node as an
indication that the media graph has been fully initialized. And I thought
that was the agreement that lead to the commit 9832e155f1ed ("[media]
media-device: split media initialization and registration").

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat