Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hello Bingbu,

On Monday, 17 December 2018 05:14:44 EET Bingbu Cao wrote:
> On 12/14/2018 06:24 AM, Laurent Pinchart wrote:
> > On Wednesday, 12 December 2018 06:55:53 EET Bingbu Cao wrote:
> >> On 12/11/2018 09:43 PM, Laurent Pinchart wrote:
> >>> On Tuesday, 11 December 2018 15:34:49 EET Laurent Pinchart wrote:
> >>>> On Wednesday, 5 December 2018 02:30:46 EET Mani, Rajmohan wrote:
> >>>> 
> >>>> [snip]
> >>>> 
> >>>>> I can see a couple of steps missing in the script below.
> >>>>> (https://lists.libcamera.org/pipermail/libcamera-devel/2018-November/0
> >>>>> 00040.html)
> >>>>> 
> >>>>>   From patch 02 of this v7 series "doc-rst: Add Intel IPU3
> >>>>>   documentation", under section "Configuring ImgU V4L2 subdev for
> >>>>>   image processing"...
> >>>>> 
> >>>>> 1. The pipe mode needs to be configured for the V4L2 subdev.
> >>>>> 
> >>>>> Also the pipe mode of the corresponding V4L2 subdev should be set as
> >>>>> desired (e.g 0 for video mode or 1 for still mode) through the control
> >>>>> id 0x009819a1 as below.
> >>>>> 
> >>>>> e.g v4l2n -d /dev/v4l-subdev7 --ctrl=0x009819A1=1
> >>>> 
> >>>> I assume the control takes a valid default value ? It's better to set
> >>>> it explicitly anyway, so I'll do so.
> >> 
> >> The video mode is set by default. If you want to set to still mode or
> >> change mode, you need set the subdev control.
> >> 
> >>>>> 2. ImgU pipeline needs to be configured for image processing as below.
> >>>>> 
> >>>>> RAW bayer frames go through the following ISP pipeline HW blocks to
> >>>>> have the processed image output to the DDR memory.
> >>>>> 
> >>>>> RAW bayer frame -> Input Feeder -> Bayer Down Scaling (BDS) ->
> >>>>> Geometric Distortion Correction (GDC) -> DDR
> >>>>> 
> >>>>> The ImgU V4L2 subdev has to be configured with the supported
> >>>>> resolutions in all the above HW blocks, for a given input resolution.
> >>>>> 
> >>>>> For a given supported resolution for an input frame, the Input Feeder,
> >>>>> Bayer Down Scaling and GDC blocks should be configured with the
> >>>>> supported resolutions. This information can be obtained by looking at
> >>>>> the following IPU3 ISP configuration table for ov5670 sensor.
> >>>>> 
> >>>>> https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+
> >>>>> /master/baseboard-poppy/media-libs/cros-camera-hal-configs-poppy/
> >>>>> files/gcss/graph_settings_ov5670.xml
> >>>>> 
> >>>>> For the ov5670 example, for an input frame with a resolution of
> >>>>> 2592x1944 (which is input to the ImgU subdev pad 0), the corresponding
> >>>>> resolutions for input feeder, BDS and GDC are 2592x1944, 2592x1944 and
> >>>>> 2560x1920 respectively.
> >>>> 
> >>>> How is the GDC output resolution computed from the input resolution ?
> >>>> Does the GDC always consume 32 columns and 22 lines ?
> >> 
> >> All the intermediate resolutions in the pipeline are determined by the
> >> actual use case, in other word determined by the IMGU input
> >> resolution(sensor output) and the final output and viewfinder resolution.
> >> BDS mainly do Bayer downscaling, it has limitation that the downscaling
> >> factor must be a value a integer multiple of 1/32.
> >> GDC output depends on the input and width should be x8 and height x4
> >> alignment.
> > 
> > Thank you for the information. This will need to be captured in the
> > documentation, along with information related to how each block in the
> > hardware pipeline interacts with the image size. It should be possible for
> > a developer to compute the output and viewfinder resolutions based on the
> > parameters of the image processing algorithms just with the information
> > contained in the driver documentation.
> > 
> >>>>> The following steps prepare the ImgU ISP pipeline for the image
> >>>>> processing.
> >>>>> 
> >>>>> 1. The ImgU V4L2 subdev data format should be set by using the
> >>>>> VIDIOC_SUBDEV_S_FMT on pad 0, using the GDC width and height obtained
> >>>>> above.
> >>>> 
> >>>> If I understand things correctly, the GDC resolution is the pipeline
> >>>> output resolution. Why is it configured on pad 0 ?
> >> 
> >> We see the GDC output resolution as the input of output system, the sink
> >> pad format is used for output and viewfinder resolutions.
> > 
> > The ImgU subdev is supposed to represent the ImgU. Pad 0 should thus be
> > the ImgU input, the format configured there should correspond to the
> > format on the connected video node, and should thus be the sensor format.
> > You can then use the crop and compose rectangles on pad 0, along with the
> > format, crop and compose rectangles on the output and viewfinder pads, to
> > configure the device. This should be fixed in the driver, and the
> > documentation should then be updated accordingly.
> 
> Hi, Laurent,
> 
> Thanks for your review.
> 
> I think it make sense for me that using Pad 0 as the ImgU input(IF).
> However, I prefer using the 2 source pads for output and viewfinder.
> It makes more sense because the output and viewfinder are independent
> output.
> 
> The whole pipeline in ImgU looks like:
> IF --> BDS --> GDC ---> OUTPUT
>                  |
>                  |-----> VF
> 
> The BDS is used to do Bayer downscaling and GDC can do cropping.

Does this mean that the main output and the viewfinder output share the same 
scaler, and that the only difference in size between the two outputs is solely 
due to cropping ?

> My understanding is that scaled size is configured on the CROP rectangle
> by COMPOSE selection target, the order seems like not aligned with the
> actual processing in ImgU if we set the crop/compose on sink pad.
> 
> Is there some rules for the order of the configuration in the subdev API?
> Could I use crop selection based on the scaled size?

Please see figure 4.6 in https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/
dev-subdev.html. Scaling is configured on the sink pad through the crop and 
compose rectangles, while the source crop rectangle is used to perform 
cropping on the output. If you have a single scaler in the hardware pipeline 
you can thus configure it on the sink pad, with output and viewfinder separate 
cropping configure on the source pad.

> >>>>> 2. The ImgU V4L2 subdev cropping should be set by using the
> >>>>> VIDIOC_SUBDEV_S_SELECTION on pad 0, with V4L2_SEL_TGT_CROP as the
> >>>>> target, using the input feeder height and width.
> >>>>> 
> >>>>> 3. The ImgU V4L2 subdev composing should be set by using the
> >>>>> VIDIOC_SUBDEV_S_SELECTION on pad 0, with V4L2_SEL_TGT_COMPOSE as the
> >>>>> target, using the BDS height and width.
> >>>>> 
> >>>>> Once these 2 steps are done, the raw bayer frames can be input to the
> >>>>> ImgU V4L2 subdev for processing.
> >>>> 
> >>>> Do I need to capture from both the output and viewfinder nodes ? How
> >>>> are they related to the IF -> BDS -> GDC pipeline, are they both fed
> >>>> from the GDC output ? If so, how does the viewfinder scaler fit in that
> >>>> picture ?
> >> 
> >> The output capture should be set, the viewfinder can be disabled.
> >> The IF and BDS are seen as crop and compose of the imgu input video
> >> device. The GDC is seen as the subdev sink pad and OUTPUT/VF are source
> >> pads.
> > 
> > The GDC is the last block in the pipeline according to the information
> > provided above. How can it be seen as the subdev sink pad ? That doesn't
> > make sense to me. I'm not asking for the MC graph to expose all internal
> > blocks of the ImgU, but if you want to retain a single subdev model, the
> > format on the sink pad needs to correspond to what is provided to the
> > ImgU. Please see figure 4.6 of
> > https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/dev-subdev.html for
> > more information regarding how you can use the sink crop, sink compose
> > and source crop rectangles.

[snip]

-- 
Regards,

Laurent Pinchart