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

From: Hans Verkuil <hverkuil@xs4all.nl>
To: Sakari Ailus <sakari.ailus@linux.intel.com>,
	"Qiu, Tian Shu" <tian.shu.qiu@intel.com>
Cc: Javier Martinez Canillas <javierm@redhat.com>,
	Bing Bu Cao <bingbu.cao@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	"Zheng, Jian Xu" <jian.xu.zheng@intel.com>,
	"Zhi, Yong" <yong.zhi@intel.com>,
	"Cao, Bingbu" <bingbu.cao@intel.com>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: Re: [PATCH] media: intel-ipu3: cio2: register the mdev on v4l2 async notifier complete
Date: Thu, 27 Sep 2018 12:09:43 +0200	[thread overview]
Message-ID: <5c6944ec-ee1f-8be6-3eff-2c65fd888222@xs4all.nl> (raw)
In-Reply-To: <20180904064605.6prcawieb4ooxtyl@paasikivi.fi.intel.com>

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.
>> 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.
> 
> In general that doesn't happen as the sensors are typically registered
> early during system boot.
> 
> 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.
> 

Today we have no userspace mechanism to deal with partially initialized topologies.
Instead if parts fails to come up we shouldn't register any media device and
instead (once we discover that something is broken) tear everything down.

In fact, video/subdev/media devices shouldn't be registered until everything is
complete.

I know we want to allow for partial bring up as well, and I fully agree with that,
but in that case someone needs to write an RFC with a proposal how userspace should
handle this.

We've discussed this in the past, but I have not seen such an RFC.

So until we add support for partial bringup I think this patch does the right
thing since otherwise this is out-of-spec.

Regards,

	Hans