RE: [PATCH v5] media: imx208: Add imx208 camera sensor driver - Chen, Ping-chung

From: "Chen, Ping-chung" <ping-chung.chen@intel.com>
To: "Yeh, Andy" <andy.yeh@intel.com>,
	Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Sakari Ailus <sakari.ailus@iki.fi>,
	"tfiga@chromium.org" <tfiga@chromium.org>,
	"sylwester.nawrocki@gmail.com" <sylwester.nawrocki@gmail.com>,
	linux-media <linux-media@vger.kernel.org>,
	"Lai, Jim" <jim.lai@intel.com>,
	"grundler@chromium.org" <grundler@chromium.org>,
	"Mani, Rajmohan" <rajmohan.mani@intel.com>
Subject: RE: [PATCH v5] media: imx208: Add imx208 camera sensor driver
Date: Thu, 27 Sep 2018 03:19:07 +0000	[thread overview]
Message-ID: <5E40A82D0551C84FA2888225EDABBE093FACDA2D@PGSMSX105.gar.corp.intel.com> (raw)
In-Reply-To: <8E0971CCB6EA9D41AF58191A2D3978B61D7A567A@PGSMSX111.gar.corp.intel.com>

Hi,

>-----Original Message-----
>From: Yeh, Andy 
>Sent: Wednesday, September 26, 2018 11:19 PM
>To: Sakari Ailus <sakari.ailus@linux.intel.com>; Chen, Ping-chung <ping-chung.chen@intel.com>

>Hi Sakari, PC,

>sensors that do need >digital gain applied, too --- assuming it'd be 
>combined with the TRY_EXT_CTRLS rounding flags.
>>
>> There might be many kinds of discrete DG formats. For imx208, DG=2^n, 
>> but for other sensors, DG could be 2*n, 5*n, or other styles. If HAL 
>> needs to
>
>I guess the most common is multiplication and a bit shift (by e.g. 8), e.g.
>multiplying the value by a 16-bit number with a 8-bit fractional part. 
>The
>imx208 apparently lacks the multiplication and only has the bit shift.
>
>Usually there's some sort of technical reason for the choice of the 
>digital gain implementation and therefore I expect at least the vast 
>majority of the implementations to be either of the two.

>We shall ensure the expansibility of this architecture to include other kind of styles in the future. Is this API design architecture-wise ok?

Indeed. Seems it is hard to cover all rules and HAL needs complex flow to judge the DG value.
Hi Sakari, could you provide an example that how HAL uses the modified interface to set available digital gain?

>
>> cover all cases, kernel will have to update more information to this 
>> control. Another problem is should HAL take over the SMIA calculation?
>> If so, kernel will also need to update SMIA parameters to user space 
>> (or create an addition filed for SMIA in the configuration XML file).
>
>The parameters for the analogue gain model should come from the driver, yes.
>We do not have controls for that purpose but they can (and should) be added.
>

>How about still follow PC's proposal to implement in XML? It was in IQ tuning file before which is in userspace. Even I proposed to PC to study with ICG SW team whether this info could be retrieved from 3A algorithm.

Hi Andy, because we has to use total gain instead of AG in 3A for the WA, our tuning data of imx208 will not include SMIA of AG anymore. 
So HAL has no way to retrieve correct SMIA parameters of AG from 3A.

Thanks,
PC Chen

>--
>Regards,
>
>Sakari Ailus
>sakari.ailus@linux.intel.com