From patchwork Fri Feb 17 15:04:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Zabel X-Patchwork-Id: 9580063 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E00EB6042F for ; Fri, 17 Feb 2017 15:06:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CFECF28236 for ; Fri, 17 Feb 2017 15:06:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C476E286A9; Fri, 17 Feb 2017 15:06:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 4AF6428236 for ; Fri, 17 Feb 2017 15:06:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Mime-Version:References:In-Reply-To: Date:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jWnKcEnB7kdiDZpWroDHY4GQaX/cRuuQPEz9HpmqnFM=; b=REzl4uLUGNO4gO 9pASljNtM6FPmNyG6eoFEreStYfz8AwT4yqBGlB7TEE5DsiZlmvamSzxbHyTQxSvHwkPqfs/DQ/5t cwx07mJpn+UjjT59Gq2rRSWfwsFzKlBJRSKo6fFxghwzvMPbv4JTREnm6zWe38cMTUqai0Q5kZjDT fAOJr4JfrzFrgFiKL7uGXqz3OghMEx0kz4hkKsiC8c1sWh5d4fFWwRmPDgIOiVBAVN6XrYt6fJ8DP cxKn5+apGxJwl6bqzVAKhnlq4U0hxvlUSOh8pFTu/A+QBU8dtglDRxvs84q0xwDUleF4H68TLYWve qFIbtwG9zB+bqZupAYvg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cek6i-0004G4-RV; Fri, 17 Feb 2017 15:06:00 +0000 Received: from metis.ext.pengutronix.de ([2001:67c:670:201:290:27ff:fe1d:cc33]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cek6H-0003cL-Le for linux-arm-kernel@lists.infradead.org; Fri, 17 Feb 2017 15:05:35 +0000 Received: from lupine.hi.pengutronix.de ([2001:67c:670:100:3ad5:47ff:feaf:1a17] helo=lupine) by metis.ext.pengutronix.de with esmtp (Exim 4.84_2) (envelope-from ) id 1cek5R-0007Rq-H7; Fri, 17 Feb 2017 16:04:41 +0100 Message-ID: <1487343870.3107.95.camel@pengutronix.de> Subject: Re: [PATCH v4 00/36] i.MX Media Driver From: Philipp Zabel To: Sakari Ailus Date: Fri, 17 Feb 2017 16:04:30 +0100 In-Reply-To: <20170217122213.GQ16975@valkosipuli.retiisi.org.uk> References: <1487211578-11360-1-git-send-email-steve_longerbeam@mentor.com> <20170216222006.GA21222@n2100.armlinux.org.uk> <923326d6-43fe-7328-d959-14fd341e47ae@gmail.com> <1487331818.3107.46.camel@pengutronix.de> <20170217122213.GQ16975@valkosipuli.retiisi.org.uk> X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2001:67c:670:100:3ad5:47ff:feaf:1a17 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170217_070533_898122_54F2B2AF X-CRM114-Status: GOOD ( 28.38 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, andrew-ct.chen@mediatek.com, minghsiu.tsai@mediatek.com, nick@shmanahar.org, songjun.wu@microchip.com, hverkuil@xs4all.nl, Steve Longerbeam , pavel@ucw.cz, robert.jarzmik@free.fr, devel@driverdev.osuosl.org, markus.heiser@darmarIT.de, laurent.pinchart+renesas@ideasonboard.com, shuah@kernel.org, Russell King - ARM Linux , geert@linux-m68k.org, Steve Longerbeam , linux-media@vger.kernel.org, devicetree@vger.kernel.org, sakari.ailus@linux.intel.com, arnd@arndb.de, mchehab@kernel.org, bparrot@ti.com, robh+dt@kernel.org, horms+renesas@verge.net.au, tiffany.lin@mediatek.com, linux-arm-kernel@lists.infradead.org, niklas.soderlund+renesas@ragnatech.se, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, jean-christophe.trotin@st.com, kernel@pengutronix.de, fabio.estevam@nxp.com, shawnguo@kernel.org, sudipm.mukherjee@gmail.com Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, 2017-02-17 at 14:22 +0200, Sakari Ailus wrote: > Hi Philipp, Steve and Russell, > > On Fri, Feb 17, 2017 at 12:43:38PM +0100, Philipp Zabel wrote: > > On Thu, 2017-02-16 at 14:27 -0800, Steve Longerbeam wrote: > > > > > > On 02/16/2017 02:20 PM, Russell King - ARM Linux wrote: > > > > On Wed, Feb 15, 2017 at 06:19:02PM -0800, Steve Longerbeam wrote: > > > >> In version 4: > > > > > > > > With this version, I get: > > > > > > > > [28762.892053] imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000000 > > > > [28762.899409] ipu1_csi0: pipeline_set_stream failed with -110 > > > > > > > > > > Right, in the imx219, on exit from s_power(), the clock and data lanes > > > must be placed in the LP-11 state. This has been done in the ov5640 and > > > tc358743 subdevs. > > > > > > If we want to bring in the patch that adds a .prepare_stream() op, > > > the csi-2 bus would need to be placed in LP-11 in that op instead. > > > > > > Philipp, should I go ahead and add your .prepare_stream() patch? > > > > I think with Russell's explanation of how the imx219 sensor operates, > > we'll have to do something before calling the sensor s_stream, but right > > now I'm still unsure what exactly. > > Indeed there appears to be no other way to achieve the LP-11 state than > going through the streaming state for this particular sensor, apart from > starting streaming. > > Is there a particular reason why you're waiting for the transmitter to > transfer to LP-11 state? That appears to be the last step which is done in > the csi2_s_stream() callback. > > What the sensor does next is to start streaming, and the first thing it does > in that process is to switch to LP-11 state. > > Have you tried what happens if you simply drop the LP-11 check? To me that > would seem the right thing to do. Removing the wait for LP-11 alone might not be an issue in my case, as the TC358743 is known to be in stop state all along. So I just have to make sure that the time between s_stream(csi2) starting the receiver and s_stream(tc358743) causing LP-11 to be changed to the next state is long enough for the receiver to detect LP-11 (which I really can't, I just have to pray I2C transmissions are slow enough). The problems start if we have to enable the D-PHY and deassert resets either before the sensor enters LP-11 state or after it already started streaming, because we don't know when the sensor drives that state on the bus. The latter case I is simulated easily by again changing the order so that the "sensor" (tc358743) is enabled before the CSI-2 receiver D-PHY initialization. The result is that captures time out, presumably because the receiver never entered HS mode because it didn't see LP-11. The PHY_STATE register contains 0x200, meaning RXCLKACTIVEHS (which we should wait for in step 7.) is never set. I tried to test the former by instead modifying the tc358743 driver a bit: ----------8<---------- ---------->8---------- That should enable the CSI-2 Tx and put it in LP-11 only after the CSI-2 receiver is enabled, right before starting streaming. That did seem to work the few times I tested, but I have no idea how this will behave with other chips that do something else to the bus while not streaming, and whether it is ok to enable the CSI right after the sensor without waiting for the CSI-2 bus to settle. regards Philipp diff --git a/drivers/media/i2c/tc358743.c b/drivers/media/i2c/tc358743.c index 39d4cdd328c0f..43df80903215b 100644 --- a/drivers/media/i2c/tc358743.c +++ b/drivers/media/i2c/tc358743.c @@ -1378,8 +1378,6 @@ static int tc358743_s_dv_timings(struct v4l2_subdev *sd, state->timings = *timings; enable_stream(sd, false); - tc358743_set_pll(sd); - tc358743_set_csi(sd); return 0; } @@ -1469,6 +1467,11 @@ static int tc358743_g_mbus_config(struct v4l2_subdev *sd, static int tc358743_s_stream(struct v4l2_subdev *sd, int enable) { + if (enable) { + tc358743_set_pll(sd); + tc358743_set_csi(sd); + tc358743_set_csi_color_space(sd); + } enable_stream(sd, enable); if (!enable) { /* Put all lanes in PL-11 state (STOPSTATE) */ @@ -1657,9 +1660,6 @@ static int tc358743_set_fmt(struct v4l2_subdev *sd, state->vout_color_sel = vout_color_sel; enable_stream(sd, false); - tc358743_set_pll(sd); - tc358743_set_csi(sd); - tc358743_set_csi_color_space(sd); return 0; }