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

["Mani, Rajmohan" <rajmohan.mani@intel.com>]

Hi Laurent,

> > Hi Rajmohan,
> >
> > On Tuesday, 4 December 2018 18:07:16 EET Mani, Rajmohan wrote:
> > > >> On Thursday, 29 November 2018 21:51:32 EET Tomasz Figa wrote:
> > > >>> On Thu, Nov 29, 2018 at 6:43 AM Laurent Pinchart wrote:
> > > >>>> On Tuesday, 30 October 2018 00:22:54 EET Yong Zhi wrote:
> > > >>
> > > >> [snip]
> > > >>
> > > >>>>> 1. Link pad flag of video nodes (i.e. ipu3-imgu 0 output) need
> > > >>>>> to be enabled prior to the test.
> > > >>>>> 2. Stream tests are not performed since it requires
> > > >>>>> pre-configuration for each case.
> > > >>>>
> > > >>>> And that's a bit of an issue. I've tested the driver with a
> > > >>>> small script based on media-ctl to configure links and yavta to
> > > >>>> interface with the video nodes, and got the following oops:
> >
> > [snip]
> >
> > > >>>> The script can be found at
> > > >>>> https://lists.libcamera.org/pipermail/libcamera-devel/2018-Nove
> > > >>>> mb
> > > >>>> e
> > > >>>> r/000040.html.
> > > >>>>
> > > >>>> I may be doing something wrong (and I probably am), but in any
> > > >>>> case, the driver shouldn't crash. Could you please have a look ?
> > > >>>
> > > >>> It looks like the driver doesn't have the default state
> > > >>> initialized correctly somewhere and it ends up using 0 as the
> > > >>> divisor in some calculation? Something to fix indeed.
> > > >>
> > > >> That's probably the case. I'll trust Intel to fix that in v8 :-)
> > > >
> > > > Ack.
> > >
> > > Thanks for catching this.
> > > I was able to reproduce this error and I see that error handling is
> > > missing, leading to the panic.
> > >
> > > https://git.linuxtv.org/sailus/media_tree.git/tree/drivers/media
> > > /pci/intel/ipu3/ipu3-css-params.c?h=ipu3-v7&id=
> > > 19cee7329ca2d0156043cac6afcde535e93310af#n433
> > >
> > > is where the -EINVAL is returned.
> > >
> > > Setting the return type as int for the following function and all
> > > its callers to use the return value properly to error out, makes the
> > > panic go away.
> >
> > I assume that I will still not be able to process frames through the
> > pipe then, as I'll get an error :-) Could you tell me why the
> > configuration is incorrect and how I can fix it ?
> >
> 
> :)
> Let me look into this more and get back.
> Thanks for your patience.

I can see a couple of steps missing in the script below.
(https://lists.libcamera.org/pipermail/libcamera-devel/2018-November/000040.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

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.

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.

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.

> Raj
> 
> > > ipu3_css_osys_calc_frame_and_stripe_params()
> > >
> > > Will include the fix in v8.
> >
> > Thank you.
> >
> > > Thanks for catching this.
> >
> > You're welcome.
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
> >
> >