Hi Sakari, In patch v6, it added the code for DG v4l2 control with integer menu type. IMX208 only has 5 steps (1x, 2x, 4x, 8x, 16x) of discrete digital gain. The imx208 driver includes the menu with 5 steps of DG and needs to notify HAL to select a suitable digital gain value from the menu by 3A result. Share the chromium link for reference. https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1297101 Thanks, Shawn -----Original Message----- From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> Sent: Thursday, October 4, 2018 11:58 PM To: Chen, Ping-chung <ping-chung.chen@xxxxxxxxx> Cc: Yeh, Andy <andy.yeh@xxxxxxxxx>; Ricardo Ribalda Delgado <ricardo.ribalda@xxxxxxxxx>; Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>; Sakari Ailus <sakari.ailus@xxxxxx>; tfiga@xxxxxxxxxxxx; sylwester.nawrocki@xxxxxxxxx; linux-media <linux-media@xxxxxxxxxxxxxxx>; Lai, Jim <jim.lai@xxxxxxxxx>; grundler@xxxxxxxxxxxx; Mani, Rajmohan <rajmohan.mani@xxxxxxxxx> Subject: Re: [PATCH v5] media: imx208: Add imx208 camera sensor driver Hi Ping-chung, On Thu, Sep 27, 2018 at 03:19:07AM +0000, Chen, Ping-chung wrote: > Hi, > > >-----Original Message----- > >From: Yeh, Andy > >Sent: Wednesday, September 26, 2018 11:19 PM > >To: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>; Chen, Ping-chung > ><ping-chung.chen@xxxxxxxxx> > > >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? It'll require more user space code no matter how you'd implement this. Thinking this again, I don't think you'd be doing harm by resorting to an integer menu here. It'll take some more time to get a decent API to provide information on the units etc. to the user space. > >> 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. Ideally the driver would be able to provide enough information here to the user space to work with it. This needs improvement going forward, but in a way that is generic enough. -- Kind regards. Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
