Hi Tomasz, Hans, > Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset > > On 12/05/2017 04:22 AM, Tomasz Figa wrote: > > Hi Raj, > > > > On Tue, Dec 5, 2017 at 9:13 AM, Mani, Rajmohan > <rajmohan.mani@xxxxxxxxx> wrote: > >> Hi Hans, > >> > >> Thanks for your patience and sharing your thoughts on this. > >> > >>> Subject: Re: [PATCH v4 00/12] Intel IPU3 ImgU patchset > >>> > >>> Hi Rajmohan, > >>> > >>> On 11/17/2017 03:58 AM, Mani, Rajmohan wrote: > >>>> Hi Sakari and all, > >>>> > >>>>> -----Original Message----- > >>>>> From: Zhi, Yong > >>>>> Sent: Tuesday, October 17, 2017 8:47 PM > >>>>> To: linux-media@xxxxxxxxxxxxxxx; sakari.ailus@xxxxxxxxxxxxxxx > >>>>> Cc: Zheng, Jian Xu <jian.xu.zheng@xxxxxxxxx>; Mani, Rajmohan > >>>>> <rajmohan.mani@xxxxxxxxx>; Toivonen, Tuukka > >>>>> <tuukka.toivonen@xxxxxxxxx>; Hu, Jerry W <jerry.w.hu@xxxxxxxxx>; > >>>>> arnd@xxxxxxxx; hch@xxxxxx; robin.murphy@xxxxxxx; > >>>>> iommu@lists.linux- foundation.org; Zhi, Yong <yong.zhi@xxxxxxxxx> > >>>>> Subject: [PATCH v4 00/12] Intel IPU3 ImgU patchset > >>>>> > >>>>> This patchset adds support for the Intel IPU3 (Image Processing > >>>>> Unit) ImgU which is essentially a modern memory-to-memory ISP. It > >>>>> implements raw Bayer to YUV image format conversion as well as a > >>>>> large number of other pixel processing algorithms for improving > >>>>> the image > >>> quality. > >>>>> > >>>>> Meta data formats are defined for image statistics (3A, i.e. > >>>>> automatic white balance, exposure and focus, histogram and local > >>>>> area contrast > >>>>> enhancement) as well as for the pixel processing algorithm parameters. > >>>>> The documentation for these formats is currently not included in > >>>>> the patchset but will be added in a future version of this set. > >>>>> > >>>> > >>>> Here is an update on the IPU3 documentation that we are currently > >>>> working > >>> on. > >>>> > >>>> Image processing in IPU3 relies on the following. > >>>> > >>>> 1) HW configuration to enable ISP and > >>>> 2) setting customer specific 3A Tuning / Algorithm Parameters to > >>>> achieve > >>> desired image quality. > >>>> > >>>> We intend to provide documentation on ImgU driver programming > >>>> interface > >>> to help users of this driver to configure and enable ISP HW to meet > >>> their needs. This documentation will include details on complete > >>> V4L2 Kernel driver interface and IO-Control parameters, except for > >>> the ISP internal algorithm and its parameters (which is Intel proprietary IP). > >>>> > >>>> We will also provide an user space library in binary form to help > >>>> users of this > >>> driver, to convert the public 3A tuning parameters to IPU3 algorithm > >>> parameters. This tool will be released under NDA to the users of this > driver. > >>> > >>> So I discussed this situation with Sakari in Prague during the ELCE. > >>> I am not happy with adding a driver to the kernel that needs an NDA > >>> to be usable. I can't agree to that. It's effectively the same as > >>> firmware that's only available under an NDA and we wouldn't accept such > drivers either. > >>> > >> > >> Ack > >> > >>> There are a few options: > >>> > >>> 1) Document the ISP parameters and that format they are stored in > >>> allowing for > >>> open source implementations. While this is the ideal, I suspect that this > is > >>> a no-go for Intel. > >>> > >> > >> Ack > >> > >>> 2) The driver can be used without using these ISP parameters and still > achieve > >>> 'OK' quality. I.e., it's usable for basic webcam usage under normal lighting > >>> conditions. I'm not sure if this is possible at all, though. > >>> > >> > >> This is something that we have tried and are able to do image capture > >> with the default ISP parameters using ov5670 sensor. > >> Additionally we had to set optimal values for the ov5670 sensor's > >> exposure and gain settings. By the above, I meant to say that optimal values of exposure and gain settings were set in / through the ov5670 sensor driver. > > > > Does it mean hardcoding some ov5670-specific settings in the ISP > > driver? If not, I guess it might be good enough? > No. There is no hardcoding of any sensor specific settings in the ISP driver. Camera HAL / user space applications are still expected to tune sensor Specific settings (Auto focus and Auto exposure) for 3A and parameters. Only Auto white balance is adjusted by default ISP parameters, with which this image was captured. > It doesn't look too bad, but it's hard to tell from just a single frame. I think you > can work with Sakari on this, if he also thinks it's good enough, then I am > happy with that. > Ack > > > >> > >> Please see if the following image looks to meet the 'OK' quality. > >> > >> git clone https://github.com/RajmohanMani/ipu3-misc.git > >> ipu3-misc/ov5670.jpg is the image file. > >> > >>> 3) Make the library available without requiring an NDA. > >>> > >> > >> We are also actively exploring this option to see if this can be done. > >> > >>> 4) Provide a non-NDA library (ideally open source) that achieves at > minimum > >>> the quality as described in 2: i.e. usable for basic webcam. > >>> > >> > >> I see this is the same as option 3) + open sourcing the library. > >> Open sourcing the library does not look to be an option. > >> I will reconfirm this. > > > > In my understanding, that could be quite different from option 3). The > > open source library would not have to implement all of the > > capabilities, just enough to get the "OK" quality and the implemented > > part could use some simpler algorithms not covered by IP. > > That's correct. > Thanks for the clarifications here. We will also be looking into this option. > This also solves my concerns about long-term maintenance of closed source > libraries which in my experience tend to suffer from bit-rot after the vendor no > longer sells the hardware it was written for. An open source implementation, > albeit not as powerful as the NDA version, can be added to e.g. the v4l-utils > repo (or at least at a repo on linuxtv.org) and maintained much longer and > independent of the vendor. > Ack
