Date:   Tue, 28 Feb 2023 10:23:34 +0200
From:   Sakari Ailus <sakari.ailus@linux.intel.com>
To:     "Wu, Wentong" <wentong.wu@intel.com>
Cc:     "mchehab@kernel.org" <mchehab@kernel.org>,
        "linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
        "Pandruvada, Srinivas" <srinivas.pandruvada@intel.com>,
        "pierre-louis.bossart@linux.intel.com" 
        <pierre-louis.bossart@linux.intel.com>,
        "Wang, Zhifeng" <zhifeng.wang@intel.com>,
        "Ye, Xiang" <xiang.ye@intel.com>,
        "Qiu, Tian Shu" <tian.shu.qiu@intel.com>,
        "Cao, Bingbu" <bingbu.cao@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Hans de Goede <hdegoede@redhat.com>
Subject: Re: [PATCH v2 3/3] media: pci: intel: ivsc: Add acquire/release API
 for ivsc
Message-ID: <Y/26BpJ15rpSAypW@kekkonen.localdomain>
References: <20230213022347.2480307-1-wentong.wu@intel.com>
 <20230213022347.2480307-4-wentong.wu@intel.com>
 <Y+uxbQi7seGf+adP@kekkonen.localdomain>
 <DM6PR11MB43169FAE93274F64AA7E06F28DAC9@DM6PR11MB4316.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <DM6PR11MB43169FAE93274F64AA7E06F28DAC9@DM6PR11MB4316.namprd11.prod.outlook.com>
Precedence: bulk

Hi Wentong,

On Tue, Feb 28, 2023 at 06:35:41AM +0000, Wu, Wentong wrote:
> Hi Sakari,
> 
> few questions as switching to v4l2 sub-dev framework.
> 
> > -----Original Message-----
> > From: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Sent: Wednesday, February 15, 2023 12:06 AM
> > 
> > Hi Wentong,
> > 
> > Thanks for the patchset.
> > 
> > On Mon, Feb 13, 2023 at 10:23:47AM +0800, Wentong Wu wrote:
> > > IVSC directly connects to camera sensor on source side, and on output
> > > side it not only connects ISH via I2C, but also exposes MIPI CSI-2
> > > interface to output camera sensor data. IVSC can use the camera sensor
> > > data to do AI algorithm, and send the results to ISH. On the other
> > > end, IVSC can share camera sensor to host by routing the raw camera
> > > sensor data to the exposed MIPI CSI-2 interface. But they can not work
> > > at the same time, so software APIs are defined to sync the ownership.
> > >
> > > This commit defines the interfaces between IVSC and camera sensor
> > > driver in include/linux/ivsc.h. The camera driver controls ownership
> > > of the CSI-2 link and sensor with the acquire/release APIs. When
> > > acquiring camera, lane number and link freq are also required by IVSC
> > > frame router.
> > 
> > The more I learn about this system, the more I'm inclined to think this
> > functionality should be exposed as a V4L2 sub-device. IVSC doesn't really do
> > anything to the data (as long as it directs it towards the CSI-2 receiver in the
> > SoC), but it is definitely part of the image pipeline.
> > 
> > I suppose the intended use cases assume a single instance of IVSC (as well as
> > MEI) but there can, and often are, be multiple camera sensors in the system. The
> > decision whether to request pass-through from IVCS can't be done in the camera
> > sensor driver, and should not be visible to the camera sensor driver. Exposing
> > IVSC as a V4L2 sub-device makes this trivial to address, as the IVSC driver's V4L2
> > sub-device video s_stream() operation gets called before streaming is started.
> > 
> > The information whether IVSC is found between the camera sensor and the
> > host's CSI-2 receiver (IPU in this case) should come from system firmware and
> > accessed most probably by what is called cio2-bridge at the moment.
> > 
> > The privacy status can be a V4L2 control.
> 
> This should be a control or event? If control, how user-space handle
> privacy stuff?

Changing control events generates events for the user space.

<URL:https://hverkuil.home.xs4all.nl/spec/userspace-api/v4l/dev-event.html>

> 
> For the required link freq and lane number, is v4l2 control the correct
> way to configure them? If yes, seems there is no CID value for them so
> that we should custom some CID value(link freqm, lane number, and
> privacy) for ivsc in linux/v4l2-controls.h, is this acceptable?

You should obtain these using the V4L2 fwnode interface. Please see e.g.
drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c how that driver
registers a V4L2 async sub-device and a V4L2 async notifier.

-- 
Kind regards,

Sakari Ailus