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

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
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
Date: Tue, 04 Dec 2018 18:42:20 +0200	[thread overview]
Message-ID: <3857756.QIBhGo4FK8@avalon> (raw)
In-Reply-To: <6F87890CF0F5204F892DEA1EF0D77A5981529599@fmsmsx122.amr.corp.intel.com>

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

> 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