From:   "Wu, Wentong" <wentong.wu@intel.com>
To:     Sakari Ailus <sakari.ailus@linux.intel.com>
CC:     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 0/3] media: pci: intel: ivsc: Add driver of Intel
 Visual Sensing Controller(IVSC)
Thread-Topic: [PATCH v2 0/3] media: pci: intel: ivsc: Add driver of Intel
 Visual Sensing Controller(IVSC)
Thread-Index: AQHZP1HlDdnw2M/ymEOJ6dDL3JcZGa7l1BmAgAACFoCACULV0IAABnKAgAABmnCAAAmBAIAClEUggACVcoCAAEDPMIAAIp0AgAotPRCAAGjUAIABdfkQgAAa7ACAA2oH8A==
Date:   Sun, 19 Mar 2023 13:09:24 +0000
Message-ID: <DM6PR11MB4316348D7B7AAC58DA0E62ED8D839@DM6PR11MB4316.namprd11.prod.outlook.com>
References: <ZAb2G7kqsEvrBhpG@kekkonen.localdomain>
 <DM6PR11MB4316B4F865472CA998E696FC8DB79@DM6PR11MB4316.namprd11.prod.outlook.com>
 <4c3ba301-6241-f2f4-f139-b4f4a0cd6223@redhat.com>
 <DM6PR11MB43166ADFA7D0775BA2C223C78DB59@DM6PR11MB4316.namprd11.prod.outlook.com>
 <e12fe65e-0b1f-a058-75e6-fa3e0a292c5b@redhat.com>
 <DM6PR11MB4316B02E112305F411B1A5158DB59@DM6PR11MB4316.namprd11.prod.outlook.com>
 <bb608934-23a6-213b-ab28-5dd66afd88e5@redhat.com>
 <MN2PR11MB43183F350CB575F94B1F05748DBC9@MN2PR11MB4318.namprd11.prod.outlook.com>
 <346c376e-01ed-267e-b1c8-ee42c345d03f@redhat.com>
 <DM6PR11MB4316F0F7B7A0F52FA5DF03BE8DBD9@DM6PR11MB4316.namprd11.prod.outlook.com>
 <ZBQrulUBX1rYD9EN@kekkonen.localdomain>
In-Reply-To: <ZBQrulUBX1rYD9EN@kekkonen.localdomain>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?pL1qTSJKwAppBbAU1FWfchh2tdNlJwppH2U745v0gomXonVe5IV4qyC9jop9?=
 =?us-ascii?Q?4OpBE13bwI/RaPbi/deZtm6PbRx4kHS0R8DdjRQKFRMmDHcMqop2K5kQMLA0?=
 =?us-ascii?Q?ttgTJCeffF9JcKpGS8GHtERw+WPTq5P/cjP5J9cw6fYA5i6cA2R7lUIMghLz?=
 =?us-ascii?Q?HXOO/UCXvgHAcyQJ+n46AS04lrGqJcc2+VN76GXXf5H3qxJlCzewSzl2cbDg?=
 =?us-ascii?Q?1dR8Lc6qhh+ifSbxlzF7q1R/9D6vpJOt8XZDaegH2AtIlgKiC2bUgXcAROly?=
 =?us-ascii?Q?e6098YTouyur0L0oeU6Yv04HcKWghiZIn5U9teDYI4khmRw17p0v6R8Qqua0?=
 =?us-ascii?Q?797/zxdd2OmUuCuEr2fj3n1RdhQUGDqyPtZzbgbfTFiCRtNzZgckUdEDg7t6?=
 =?us-ascii?Q?iktvEIKFfe+3tJIHhBlKaKvmO8elipjQgTlw/H8dyT8Oy0R9S86C05PuXGZL?=
 =?us-ascii?Q?zgyThqI7UY3eH4nUy4DXHuVpP+5K0iRsUXg3KixBdj62mbV0dOQGiBf45oAc?=
 =?us-ascii?Q?qkOUpKaRTptVNNb5z6PdcnHKb+fTeBSIqF1cWjrIiALU0yGPra+4f50hTZik?=
 =?us-ascii?Q?aSWUtxdhsvVlOfuJ7VIbJnyhSWCA6Bt3C6wJWz5L4S7RAbXrQxmA3qSGTwRX?=
 =?us-ascii?Q?gkXpBTRdiPKqeygzl6cG7oqrr7AQG62vsA9Kk/0EM5zmWZy1jJLLeTXNd/Pc?=
 =?us-ascii?Q?GyftJfyny1y51Vl4fxkf70adXvXcNKXSbic0+hbevZ1UugLdAM2/nlKW9h1K?=
 =?us-ascii?Q?afeU3oqk/RH7faLeWgzDw5UeoiLPT4OeyVmV9gD6S1BqKbwvawYUonFVN8cx?=
 =?us-ascii?Q?QD1GcV/fdkZSNwaacB3KGruRXrc5hj9776MVTwtPR7UcBfR/qT1MvY8BEYxo?=
 =?us-ascii?Q?qpBCOURjVBLYMWTKvyMBAnehfOoFEOrOI+84qgGSH9AdoWgI30eq8vwkJVCD?=
 =?us-ascii?Q?SUkcBIXNtP8QrXzQl6lXrIUivikXuyA3ueLlo0Ra+PE0E41QLQJoKtq/Y1Hz?=
 =?us-ascii?Q?V4mVRmKgpKTO/uZq1hfKZRA89qVjpeveMah5SuKFqq5T7OHKsWlkjMM1QTYb?=
 =?us-ascii?Q?6OxyzVr8VESNsfS+DiofVk1tVDBOTtoMP2ib6wsrKS8R8xR4AjnLQ7hL6F5M?=
 =?us-ascii?Q?qle3zgXdnJ9q2bgS37morITADwJngAlTd0icxO+t8mf7DNBektTWKimh4E7G?=
 =?us-ascii?Q?2+QJX7lVGmycYft8UvnD4aej5O/4z7QaPl22CtpH0yKR/BLDworxoExcBLVz?=
 =?us-ascii?Q?MVoAnUBQJZpOXvIi01GNW2z1vssQHEJ6ZgYxebA7Nkkqg9+6kXn56ZUMpsky?=
 =?us-ascii?Q?LB0L0TF1e6SMLe0D5yeIO/YCIp/eLjB6PtkHJjDpaxQGKET6vkBJBIkm96YU?=
 =?us-ascii?Q?OA3KarFgLmVEL5EvMGwH7ovr0vDehzmbdDvsoT73sJDsmIl/eR8wLAnt9LAS?=
 =?us-ascii?Q?lL3owuZHhJtpeqpKN8iR3UFfxOB69bIzdtDRsprtzvHRDiV5VYlpq/7OWg8S?=
 =?us-ascii?Q?tWg7zd+R1wFPwbF+i0DBwuKt78OdXP7K7LUwoO9IlC7l4DBrRjNqCqHeR+sb?=
 =?us-ascii?Q?no5yrB8g4W4WlFBn1J0PhLs42KABYydz5gC/3WTD?=
Content-Type: text/plain; charset="us-ascii"
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: ddfcb174-49c4-4102-1cf5-08db287b299a
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Mar 2023 13:09:24.6642
 (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: FEMOAhMNM8qasWi/ukz5kq+Hsse27uPcQDpVEFrhzMbCJwUhwj56l+DFM9kFPGlRi18gKI5ntsVacKWfDpltFA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7679
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: Friday, March 17, 2023 4:59 PM
>=20
> Hi Wentong,
>=20
> On Fri, Mar 17, 2023 at 07:30:19AM +0000, Wu, Wentong wrote:
> >
> >
> > > -----Original Message-----
> > > From: Hans de Goede <hdegoede@redhat.com>
> > > Sent: Thursday, March 16, 2023 5:04 PM
> > >
> > > Hi,
> > >
> > > On 3/16/23 03:58, Wu, Wentong wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Hans de Goede <hdegoede@redhat.com>
> > > >> Sent: Thursday, March 9, 2023 11:24 PM
> > > >>
> > > >> <re-added the previous Cc list, which I dropped because of the
> > > >> large
> > > >> attachments>
> > > >>
> > > >> Hi Wentong,
> > > >>
> > > >> On 3/9/23 15:29, Wu, Wentong wrote:
> > > >>> Hi Hans,
> > > >>>
> > > >>> Thanks
> > > >>>
> > > >>> And AFAICT, there is no IVSC device on your Dell Latitude 9420
> > > >>> where the
> > > >> platform is based on TGL instead of ADL, and I have never heard
> > > >> IVSC runs on TGL,  if no IVSC, INT3472 will control sensor's power=
.
> > > >>> And I will double confirm with people who know dell product well
> tomorrow.
> > > >>
> > > >> Ah, I was under the impression that there was an IVSC there becaus=
e:
> > > >>
> > > >> 1. The sensor driver for the used sensor (tries to) poke the IVSC =
2.
> > > >> Things did not work without building the IVSC drivers, but that mi=
ght
> > > >>    be due to a dependency on the LCJA GPIO expander instead
> > > >
> > > > Below is your dmesg log, the required SPI controller for IVSC isn't=
 here.
> > > >
> > > > [   35.538114] ljca 2-6:1.0: acked sem wait timed out ret:0 timeout=
:20
> ack:0
> > > > [   35.538129] ljca 2-6:1.0: MNG_ENUM_SPI failed ret:-110 len:7 num=
:0
> > > > [   35.538621] ljca 2-6:1.0: LJCA USB device init success
> > > > [   35.538776] usbcore: registered new interface driver ljca
> > > >
> > > > Also I checked your SSDT, there is no IVSC device and the sensor
> > > > device depends on
> > > > INT3472 instead of IVSC device as on my setup.
> > >
> > > Ack.
> > >
> > > >> But you might very well be right, that would also explain the
> > > >> "intel vsc not
> > > ready"
> > > >> messages in dmesg.
> > > >>
> > > >> If with the IVSC case the IVSC controls the power to the sensor
> > > >> too, then another option might be to model the I2C-switch + the
> > > >> power-control as a powerdown GPIO for the sensor, which most
> > > >> sensor
> > > drivers already try to use.
> > > >> The advantage of doing this would be that GPIO lookups can
> > > >> reference the GPIO provider + consumer by device-name so then we
> > > >> don't need to have both devices instantiated at the time of
> > > >> adding the GPIO lookup.   And in that case we could e.g. add the l=
ookup
> > > >> before registering the I2C controller.
> > > >
> > > > Can we add IVSC device to acpi_honor_dep_ids, so that when
> > > > everything is done during mei_ace probe,
> > > > acpi_dev_clear_dependencies can make sensor
> > > start probe?
> > >
> > > Does the sensor ACPI device node have an ACPI _DEP on the IVSC device=
 ?
> >
> > Yes,
> >
> > >
> > > If yes, then yes we can add the IVSC device to acpi_honor_dep_id and
> > > make mei_ace probe call acpi_dev_clear_dependencies().
> >
> > But I prefer the powerdown gpio model, because we have to follow the
> > commands sequences as below which is required by firmware, runtime pm i=
s
> hard to achieve this.
>=20
> How so?

But we have to find a way to download commands to firmware following below
sequence before writing camera sensor's registers to start camera sensor's =
steam.

BR,
Wentong

>=20
> I don't insist on the runtime PM based solution but I'd rather not have c=
hanges
> to virtually all sensor drivers --- this is an external chip to them.
>=20
> > +	/* switch camera sensor ownership to host */
> > +	ret =3D ace_set_camera_owner(ACE_CAMERA_HOST);
> > +	if (ret)
> > +		goto error;
> > +
> > +	/* switch CSI-2 link to host */
> > +	ret =3D csi_set_link_owner(CSI_LINK_HOST, callback, context);
> > +	if (ret)
> > +		goto release_camera;
> > +
> > +	/* configure CSI-2 link */
> > +	ret =3D csi_set_link_cfg(nr_of_lanes, link_freq);
> > +	if (ret)
> > +		goto release_csi;
>=20
> --
> Kind regards,
>=20
> Sakari Ailus