From:   "Wu, Wentong" <wentong.wu@intel.com>
To:     Sakari Ailus <sakari.ailus@linux.intel.com>
CC:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        Hans de Goede <hdegoede@redhat.com>,
        "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>
Subject: RE: [PATCH v2 3/3] media: pci: intel: ivsc: Add acquire/release API
 for ivsc
Thread-Topic: [PATCH v2 3/3] media: pci: intel: ivsc: Add acquire/release API
 for ivsc
Thread-Index: AQHZP1Hum/btr+ww/kiynrKipZgIc67OndeAgAEcQoCAAAvHAIAC4+MggABa/gCABDg8QIAAUUiAgAAFpoA=
Date:   Mon, 20 Feb 2023 08:57:47 +0000
Message-ID: <DM6PR11MB4316B3105394F258D8205DBC8DA49@DM6PR11MB4316.namprd11.prod.outlook.com>
References: <20230213022347.2480307-1-wentong.wu@intel.com>
 <20230213022347.2480307-4-wentong.wu@intel.com>
 <Y+uxbQi7seGf+adP@kekkonen.localdomain>
 <0457831e-a347-8278-01fe-52c011759d90@redhat.com>
 <Y+ypwnRVwReIEjUo@pendragon.ideasonboard.com>
 <DM6PR11MB43166A8D4225C8460C29803B8DA19@DM6PR11MB4316.namprd11.prod.outlook.com>
 <Y+9iwMbW0kk6eJKe@pendragon.ideasonboard.com>
 <DM6PR11MB4316A36ACA077501B92C1D0E8DA49@DM6PR11MB4316.namprd11.prod.outlook.com>
 <Y/MxGRrKsgVat3zZ@kekkonen.localdomain>
In-Reply-To: <Y/MxGRrKsgVat3zZ@kekkonen.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?dEEV/vpQgaAo+U3XuITiH/URuxMjIfjIdE+AxoH1fEteg5TbWXhF6XGLT6?=
 =?iso-8859-1?Q?jPalNOyj+zgGFl7nhdm4PG/P5Wt872p7bUAX1IP4c6TYaYZLi/cqfehd5I?=
 =?iso-8859-1?Q?9IRBycqptbsVshrLP4Y3RP9Hh4HHArBMHmk7EMwgduM7lyq42cV4ZPAshm?=
 =?iso-8859-1?Q?IaVuc2Xfc+ec6FtWGlqvxVw+LmPlPEDKzZoJmyo0zqBRjsx5y+GvIHf5v/?=
 =?iso-8859-1?Q?/YfRXkgaGC0awYiMEJIujiwse7RVgE7Cc/nDilsAxI1DG8Bg02wCg0UxgM?=
 =?iso-8859-1?Q?Bpszs78m6KhrHRKa5djsoFX+PHIuQaPY0LRPzWBXRea2sIXcEOwccKQr5T?=
 =?iso-8859-1?Q?U/B0NOIRH72CwIOc4N+b435QZnDo6yyQaI0yafkstiSdpGp2AH5apvAnId?=
 =?iso-8859-1?Q?Pgunsa6NBvorO8N6KbE7QboeUYtGIv3oKG1V1aA375T3wfREdrAz9NJgfS?=
 =?iso-8859-1?Q?JNx58XZMmyTifqy/sQvo65bC6WDxMhZVIeJnINYegxYKUbr9tjih7vZIS9?=
 =?iso-8859-1?Q?dYGyCv8m/JHJ1X8DPCrWEyqH5TgMPxR1bcuAKhT/RvPy1Pl/TGf9KAwAPh?=
 =?iso-8859-1?Q?bepk7+r7qLI2cdw/ViRwwqiNPLD+H1T5vI/LIUwqEtbt2FZFaX1syngdGA?=
 =?iso-8859-1?Q?U3Yf1ho4NCSoGJHEwr9SoKdLxh/bwic+cs8iZkvSBCw+4duRy+yatw/iZ/?=
 =?iso-8859-1?Q?Z6PwABbOtxWWkb7wHiSsr2P0WZHo5KdRsaAsd6Ia+Pb6FiNbdBnlDZADVd?=
 =?iso-8859-1?Q?zYy9Qxc3x6mhfQIa8j8gUUxfCUOC7phV8AGxBmyyOICa40VgXnIjMdn/ZV?=
 =?iso-8859-1?Q?t77mExuPcF68VG83zPpvctIyS6NNNSyDkh9xQ8qUjHtxIbybp/cVqEa4Qt?=
 =?iso-8859-1?Q?CV0ZrDYyYPtsu89l7vJBO5Gj3RAqIkmY87MfLBctKNg2I1jn7VSvpqB5Lj?=
 =?iso-8859-1?Q?cyeGL4FEQF+9h6fuzOb3u5zYBgWydSU8/nEc7M+fePxsPZ5kzy7UAs9tc/?=
 =?iso-8859-1?Q?H1IJ4NhlyydSyaojvVSMVqGplaz7I+IeTXW/YCzwxT3QEHwYljozlF9wNt?=
 =?iso-8859-1?Q?kY8rJm/4AHFuUaS9mp7jtf1riMATyWUPizNWJ0E7jp8bdGdOnOIc9SQlyY?=
 =?iso-8859-1?Q?N7QN0YinbDLwT53R0Yid1M+1LAdMO7GDk+Fb6CuDKso9BAGxIsPyTa29hE?=
 =?iso-8859-1?Q?xij0AAWClLz7wCKLyexLT+CDikvykL92fpYOWl54baaa8LetmFk7TgJUUs?=
 =?iso-8859-1?Q?ru22fNqFRdc8t1gA1HYBso5nDeXCnmg8/O/G77/fGcLV5kqvhgOM57cB7U?=
 =?iso-8859-1?Q?sE4EJ6Kz+8OoErIhEmqiMXOaL2IxwWMmEyggOZoC+OdTqVkNPoYqEHtqmX?=
 =?iso-8859-1?Q?35i4vHwea31tQefUtS+nhGBxH29RoMFMdhgWpb48UfQJyNvpzaCdduDWNF?=
 =?iso-8859-1?Q?Ss38RA8THxXHCKORu+5mZFRDWArVVw/NN+tXre1sNIq2V9SomkPbampKMo?=
 =?iso-8859-1?Q?RCYwBEcKgjoHVrbQSrSgXYWKpOluikAYw9Z3K+3vRZQZebS+0JlbPoGOgC?=
 =?iso-8859-1?Q?jNSAKKM67h4qUFS+FgHYQOPtmSdDtZbXIz0xwrGDjz6z85jlfZRdbLZjES?=
 =?iso-8859-1?Q?4CWpVN4CtVTMOqFPAxEN6igne3Cb2tTvti?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4316.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3e7eea57-c0c2-465e-74e8-08db132089ac
X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Feb 2023 08:57:47.2094
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: H1H9381Pbxf5LDNrVGjDiv95ErP+p96g7RxdaU0/hndFPQB3q5p7faRodhRHr0KYWVLqW43uha6shRlN4+eN6w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6083
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@linux.intel.com>
> Sent: Monday, February 20, 2023 4:37 PM
>=20
> Hi Wentong,
>=20
> On Mon, Feb 20, 2023 at 03:50:55AM +0000, Wu, Wentong wrote:
> >
> >
> > > -----Original Message-----
> > > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Sent: Friday, February 17, 2023 7:19 PM
> > >
> > > Hi Wentong,
> > >
> > > On Fri, Feb 17, 2023 at 06:10:22AM +0000, Wu, Wentong wrote:
> > > > On Sent: Wednesday, February 15, 2023 5:46 PM, Laurent Pinchart wro=
te:
> > > > > On Wed, Feb 15, 2023 at 10:03:29AM +0100, Hans de Goede wrote:
> > > > > > On 2/14/23 17:06, Sakari Ailus wrote:
> > > > > > > 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 resu=
lts 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-devi=
ce.
> > > > > > > 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.
> > > > > >
> > > > > > Yes I happened to discuss this exact same thing with Laurent
> > > > > > at FOSDEM and we also came to the conclusion that the IVSC
> > > > > > chip should be modeled as a V4L2 sub-device.
> > > > >
> > > > > Agreed.
> > > >
> > > > Thanks for your quick review and conclusion, I'm fresh to media
> > > > sub-system, is there any convention that I should follow to
> > > > upstream this kind of v4l2 sub-device driver so that not too much
> > > > back and forth?
> > >
> > > This is a tentative proposal, as I'm not very familiar with the
> > > hardware
> > > architecture:
> > >
> > > - The subdev should have two pads, a sink pad connected to the camera
> > >   sensor, and a source pad connected to the CSI-2 receiver in the IPU=
.
> > >
> > > - As for any new driver, the subdev driver should use the active stat=
e
> > >   managed by the v4l2-subdev core. This requires calling
> > >   v4l2_subdev_init_finalize() at probe time. See
> > >   https://git.linuxtv.org/media_tree.git/commit/?id=3Da2514b9a634a fo=
r an
> > >   example of a subdev driver converted to this mechanism.
> > >
> > > - As we're talking about CSI-2, the subdev driver should use the stre=
ams
> > >   API that was recently merged, and in particular support the
> > >   .get_frame_desc(), .set_routing(), .enable_streams() and
> > >   .disable_streams() operations.
> > >
> > > - I don't see a need to support V4L2 controls in the subdev driver, b=
ut
> > >   I may be missing something.
> > >
> > > - The driver should be validated with v4l2-compliance, part of
> > >   v4l-utils.
> > >
> >
> > Thanks for the detail, but I have one more question, during probe of
> > sensor(v4l2-sudev) driver, it will configure camera sensor connected
> > to IVSC via I2C, but before that it should acquire camera sensor's
> > ownership from IVSC, how v4l2 framework guarantee this?
>=20
> Please wrap the lines at 74 characters or so when replying.

Thanks, I will.

>=20
> Do you mean accessing the sensor via I=B2C also requires acquiring the se=
nsor
> from IVSC?

Yes

>=20
> --
> Regards,
>=20
> Sakari Ailus