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

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

Hi Laurent,

> -----Original Message-----
> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: Tuesday, December 04, 2018 8:42 AM
> To: Mani, Rajmohan <rajmohan.mani@intel.com>
> Cc: Tomasz Figa <tfiga@chromium.org>; Zhi, Yong <yong.zhi@intel.com>;
> Linux Media Mailing List <linux-media@vger.kernel.org>; Sakari Ailus
> <sakari.ailus@linux.intel.com>; Mauro Carvalho Chehab
> <mchehab@kernel.org>; Hans Verkuil <hans.verkuil@cisco.com>; Zheng, Jian
> Xu <jian.xu.zheng@intel.com>; Hu, Jerry W <jerry.w.hu@intel.com>;
> Toivonen, Tuukka <tuukka.toivonen@intel.com>; Qiu, Tian Shu
> <tian.shu.qiu@intel.com>; Cao, Bingbu <bingbu.cao@intel.com>
> Subject: Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
> 
> 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-Novemb
> > >>>> 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.

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
> 
>