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. > >> The resolution of output and viewfinder should be fixed once the >> pipeline is determined.