Re: [PATCH v3 00/11] CSI2RX support on J721E

From: Pratyush Yadav <p.yadav@ti.com>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: "Jacopo Mondi" <jacopo@jmondi.org>,
	"Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Laurent Pinchart" <laurent.pinchart@ideasonboard.com>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Nikhil Devshatwar" <nikhil.nd@ti.com>,
	"Alexandre Courbot" <acourbot@chromium.org>,
	"Arnd Bergmann" <arnd@arndb.de>, "Benoit Parrot" <bparrot@ti.com>,
	"Bert Vermeulen" <bert@biot.com>,
	"Dikshita Agarwal" <dikshita@codeaurora.org>,
	"Dongchun Zhu" <dongchun.zhu@mediatek.com>,
	"Eugen Hristev" <eugen.hristev@microchip.com>,
	"Ezequiel Garcia" <ezequiel@collabora.com>,
	"Fabio Estevam" <festevam@gmail.com>,
	"Georgi Djakov" <georgi.djakov@linaro.org>,
	"Hans Verkuil" <hverkuil-cisco@xs4all.nl>,
	"Helen Koike" <helen.koike@collabora.com>,
	"Jacopo Mondi" <jacopo+renesas@jmondi.org>,
	"Jiapeng Chong" <jiapeng.chong@linux.alibaba.com>,
	"Kieran Bingham" <kieran.bingham+renesas@ideasonboard.com>,
	"Kieran Bingham" <kieran.bingham@ideasonboard.com>,
	"Lad Prabhakar" <prabhakar.mahadev-lad.rj@bp.renesas.com>,
	"Martina Krasteva" <martinax.krasteva@intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Michael Tretter" <m.tretter@pengutronix.de>,
	"Mirela Rabulea" <mirela.rabulea@nxp.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>,
	"Paul Kocialkowski" <paul.kocialkowski@bootlin.com>,
	"Qiushi Wu" <wu000273@umn.edu>,
	"Raag Jadav" <raagjadav@gmail.com>,
	"Ricardo Ribalda" <ribalda@chromium.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Sakari Ailus" <sakari.ailus@linux.intel.com>,
	"Stanimir Varbanov" <stanimir.varbanov@linaro.org>,
	"Steve Longerbeam" <slongerbeam@gmail.com>,
	"Thomas Bogendoerfer" <tsbogend@alpha.franken.de>,
	"Tianshu Qiu" <tian.shu.qiu@intel.com>,
	"Yang Yingliang" <yangyingliang@huawei.com>,
	"Zou Wei" <zou_wei@huawei.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v3 00/11] CSI2RX support on J721E
Date: Thu, 8 Jul 2021 16:55:06 +0530	[thread overview]
Message-ID: <20210708112504.optj6xzkljrpbwsg@ti.com> (raw)
In-Reply-To: <c7ff2f59-7975-adad-d9dd-d4084eecb65b@ideasonboard.com>

On 08/07/21 11:43AM, Tomi Valkeinen wrote:
> On 08/07/2021 11:19, Jacopo Mondi wrote:
> > Hi Tomi, Pratyush,
> > 
> > On Thu, Jul 08, 2021 at 12:26:38AM +0530, Pratyush Yadav wrote:
> > > Hi Tomi,
> > > 
> > > Thanks for looking into the patches.
> > > 
> > > On 01/07/21 10:56AM, Tomi Valkeinen wrote:
> > > > Hi Pratyush,
> > > > 
> > > > On 24/06/2021 22:21, Pratyush Yadav wrote:
> > > > > Hi,
> > > > > 
> > > > > This series adds support for CSI2 capture on J721E. It includes some
> > > > > fixes to the Cadence CSI2RX driver, adds runtime PM support to OV5640
> > > > > driver, and finally adds the TI CSI2RX wrapper driver.
> > > > > 
> > > > > This series used to include the DPHY and DMA engine patches as well, but
> > > > > they have been split off to facilitate easier merging. Patch 3 is
> > > > > build-dependent on the DPHY series [0].
> > > > > 
> > > > > The DMA engine patch [1] can go in any order since that is only a run
> > > > > time dependency. Things probably won't work without it but it will still
> > > > > build fine.
> > > > > 
> > > > > Tested on TI's J721E with OV5640 sensor.
> > > > 
> > > > I applied these (csi-2 rx, phy, dma-engine) to linux-media/master, and added dts changes to add the csi2-rx. When sending the series, can you also push the branch you use for testing, as the posted patches do not include everything needed?
> > > 
> > > Please use https://github.com/prati0100/linux-next/ branch "capture"
> > > 
> > > I will include the link in the cover letter from next version onwards.
> > > 
> > > > 
> > > > Here are some notes from quick tests:
> > > > 
> > > > Capture works, but the fps is ~28.98. I would expect it to be closer to 30. Are the clocks configured correctly?
> > > 
> > > I see this as well. I figured this had something to do with the sensor.
> > 
> > Tomi you might remember your patch to change the h/vtot values which I
> > collected in a series which I never managed to bring to v1, as Hugues
> > reported it was broken for JPEG capture.
> > 
> > I'll leave it here just for reference, I admit I dropped the ball
> > rather quickly there:
> > https://patchwork.linuxtv.org/project/linux-media/cover/20201028225706.110078-1-jacopo+renesas@jmondi.org/
> > 
> > I wish I could re-test but seems I've lost the powering cable of the
> > device I used to test ov5640 :(
> 
> Yes, I'm still using my hack patch when working with OV5640. With that hack,
> on TI platforms with CAL IP, I get ~30fps. With this series on J7, I get the
> above mentioned 28.98.
> 
> It's possible my hack patch is wrong, and CAL driver is buggy, but together
> they make things right. I guess I should also try J7 without my hack patch.

I think this is a OV5640 related issue. On IMX219 I am seeing exactly 30 
fps.

> 
> If I recall right, I tested your changes but I couldn't get them to work on
> my HW.
> 
> I haven't worked on that since then, as I decided that debugging blind is
> pointless. We need someone to analyze the signals to see what OV5640 is
> sending. Or some new understanding about the OV5640 HW.
> 
>  Tomi

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.