On Tue, Nov 13, 2018 at 07:04:01PM +0800, Bing Bu Cao wrote: > > > On 11/13/2018 06:31 PM, Sakari Ailus wrote: > > Hi Bing Bu, > > > > On Mon, Nov 12, 2018 at 12:31:16PM +0800, Bing Bu Cao wrote: > >> > >> On 11/09/2018 06:09 PM, Sakari Ailus wrote: > >>> Hi Bing Bu, > >>> > >>> On Wed, Nov 07, 2018 at 12:16:47PM +0800, Bing Bu Cao wrote: > >>>> On 11/01/2018 08:03 PM, Sakari Ailus wrote: > >>>>> Hi Yong, > >>>>> > >>>>> Thanks for the update! > >>>>> > >>>>> On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > >>>>>> Hi, > >>>>>> > >>>>>> This series 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. > >>>>>> > >>>>>> The algorithm parameters need to be considered specific to a given frame > >>>>>> and typically a large number of these parameters change on frame to frame > >>>>>> basis. Additionally, the parameters are highly structured (and not a flat > >>>>>> space of independent configuration primitives). They also reflect the > >>>>>> data structures used by the firmware and the hardware. On top of that, > >>>>>> the algorithms require highly specialized user space to make meaningful > >>>>>> use of them. For these reasons it has been chosen video buffers to pass > >>>>>> the parameters to the device. > >>>>>> > >>>>>> On individual patches: > >>>>>> > >>>>>> The heart of ImgU is the CSS, or Camera Subsystem, which contains the > >>>>>> image processors and HW accelerators. > >>>>>> > >>>>>> The 3A statistics and other firmware parameter computation related > >>>>>> functions are implemented in patch 11. > >>>>>> > >>>>>> All IPU3 pipeline default settings can be found in patch 10. > >>>>>> > >>>>>> To access DDR via ImgU's own memory space, IPU3 is also equipped with > >>>>>> its own MMU unit, the driver is implemented in patch 6. > >>>>>> > >>>>>> Patch 7 uses above driver for DMA mapping operation. > >>>>>> > >>>>>> The communication between IPU3 firmware and driver is implemented with circular > >>>>>> queues in patch 8. > >>>>>> > >>>>>> Patch 9 provide some utility functions and manage IPU3 fw download and > >>>>>> install. > >>>>>> > >>>>>> The firmware which is called ipu3-fw.bin can be downloaded from: > >>>>>> > >>>>>> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > >>>>>> (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) > >>>>>> > >>>>>> Firmware ABI is defined in patches 4 and 5. > >>>>>> > >>>>>> Patches 12 and 13 are of the same file, the former contains all h/w programming > >>>>>> related code, the latter implements interface functions for access fw & hw > >>>>>> capabilities. > >>>>>> > >>>>>> Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: > >>>>>> > >>>>>> <URL:https://patchwork.kernel.org/patch/9976295/> > >>>>> I've pushed the latest set here: > >>>>> > >>>>> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > >>>>> > >>>>> You can just say the entire set depends on those going forward; the > >>>>> documentation is needed, too. > >>>>> > >>>>>> Patch 15 represents the top level that glues all of the other components together, > >>>>>> passing arguments between the components. > >>>>>> > >>>>>> Patch 16 is a recent effort to extend v6 for advanced camera features like > >>>>>> Continuous View Finder (CVF) and Snapshot During Video(SDV) support. > >>>>>> > >>>>>> Link to user space implementation: > >>>>>> > >>>>>> git clone https://chromium.googlesource.com/chromiumos/platform/arc-camera > >>>>>> > >>>>>> ImgU media topology print: > >>>>>> > >>>>>> # media-ctl -d /dev/media0 -p > >>>>>> Media controller API version 4.19.0 > >>>>>> > >>>>>> Media device information > >>>>>> ------------------------ > >>>>>> driver ipu3-imgu > >>>>>> model ipu3-imgu > >>>>>> serial > >>>>>> bus info PCI:0000:00:05.0 > >>>>>> hw revision 0x80862015 > >>>>>> driver version 4.19.0 > >>>>>> > >>>>>> Device topology > >>>>>> - entity 1: ipu3-imgu 0 (5 pads, 5 links) > >>>>>> type V4L2 subdev subtype Unknown flags 0 > >>>>>> device node name /dev/v4l-subdev0 > >>>>>> pad0: Sink > >>>>>> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > >>>>> This doesn't seem right. Which formats can be enumerated from the pad? > >>> Looking at the code, the OUTPUT video nodes have 10-bit GRBG (or a variant) > >>> format whereas the CAPTURE video nodes always have NV12. Can you confirm? > >> Hi, Sakari, > >> Yes, I think the pad_fmt should also be changed. > >> Yong, could you add some extra code for this and test? like: > >> > >> static int ipu3_v4l2_node_setup(struct imgu_device *imgu, unsigned int pipe, > >> ... > >> V4L2_PIX_FMT_NV12; > >> node->vdev_fmt.fmt.pix_mp = def_pix_fmt; > >> } > >> > >> + if (node->vdev_fmt.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) > >> + node->pad_fmt.code = MEDIA_BUS_FMT_SGRBG10_1X10; > >> + > >> > >>> If the OUTPUT video node format selection has no effect on the rest of the > >>> pipeline (device capabilities, which processing blocks are in use, CAPTURE > >>> video nodes formats etc.), I think you could simply use the FIXED media bus > >>> code for each pad. That would actually make sense: this device always works > >>> from memory to memory, and thus does not really have a pixel data bus > >>> external to the device which is what the media bus codes really are for. > >>> > >>>>>> crop:(0,0)/1920x1080 > >>>>>> compose:(0,0)/1920x1080] > >>>>> Does the compose rectangle affect the scaling on all outputs? > >>>> Sakari, driver use crop and compose targets to help set input-feeder and BDS > >>>> output resolutions which are 2 key block of whole imaging pipeline, not the > >>>> actual ending output, but they will impact the final output. > >>> Ack. Thanks for the clarification. > >>> > >>>>>> <- "ipu3-imgu 0 input":0 [] > >>>>> Are there links that have no useful link configuration? If so, you should > >>>>> set them enabled and immutable in the driver. > >>>> The enabled status of input pads is used to get which pipe that user is > >>>> trying to enable (ipu3_link_setup()), so it could not been set as immutable. > >>> But the rest of them could be, right? > >> Yes. > >>>>>> pad1: Sink > >>>>>> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > >>>>> I'd suggest to use MEDIA_BUS_FMT_FIXED here. > >>>>> > >>>>>> <- "ipu3-imgu 0 parameters":0 [] > >>>>>> pad2: Source > >>>>>> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > >>>>>> -> "ipu3-imgu 0 output":0 [] > >>>>>> pad3: Source > >>>>>> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > >>>>>> -> "ipu3-imgu 0 viewfinder":0 [] > >>>>> Are there other differences between output and viewfinder? > >>>> output and viewfinder are the main and secondary output of output system. > >>>> 'main' output is not allowed to be scaled, only support crop. secondary > >>>> output 'viewfinder' > >>>> can support both cropping and scaling. User can select different nodes > >>>> to use > >>>> as preview and capture flexibly based on the actual use cases. > >>> If there's scaling to be configured, I'd expect to see the COMPOSE target > >>> supported. > >> Actually the viewfinder is the result of scaling, that means you can not > >> do more scaling. > > How do you configure the scaling of the viewfinder currently? > We consider that the viewfinder as a secondary output, and set the format by > subdev set_fmt() directly and all pads formats will be used to find > binary and > build pipeline. Ok. Could you instead use the compose target to configure the scaling? Setting the format on the source pad would have no effect. -- Kind regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx