From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Longerbeam Subject: Re: [PATCH v5 00/39] i.MX Media Driver Date: Sun, 12 Mar 2017 21:26:41 -0700 Message-ID: References: <1489121599-23206-1-git-send-email-steve_longerbeam@mentor.com> <20170310201356.GA21222@n2100.armlinux.org.uk> <47542ef8-3e91-b4cd-cc65-95000105f172@gmail.com> <20170312195741.GS21222@n2100.armlinux.org.uk> <20170312202240.GT21222@n2100.armlinux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170312202240.GT21222@n2100.armlinux.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Russell King - ARM Linux Cc: mark.rutland@arm.com, andrew-ct.chen@mediatek.com, minghsiu.tsai@mediatek.com, sakari.ailus@linux.intel.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, geert@linux-m68k.org, linux-media@vger.kernel.org, devicetree@vger.kernel.org, kernel@pengutronix.de, 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, p.zabel@pengutronix.de, fabio.estevam@nxp.com, shawnguo@kernel.org, sudipm.mukherjee@gmail.com List-Id: devicetree@vger.kernel.org On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote: > On Sun, Mar 12, 2017 at 01:05:06PM -0700, Steve Longerbeam wrote: >> >> >> On 03/12/2017 12:57 PM, Russell King - ARM Linux wrote: >>> On Sat, Mar 11, 2017 at 04:30:53PM -0800, Steve Longerbeam wrote: >>>> If it's too difficult to get the imx219 csi-2 transmitter into the >>>> LP-11 state on power on, perhaps the csi-2 receiver can be a little >>>> more lenient on the transmitter and make the LP-11 timeout a warning >>>> instead of error-out. >>>> >>>> Can you try the attached change on top of the version 5 patchset? >>>> >>>> If that doesn't work then you're just going to have to fix the bug >>>> in imx219. >>> >>> That patch gets me past that hurdle, only to reveal that there's another >>> issue: >> >> Yeah, ipu_cpmem_set_image() failed because it doesn't recognize the >> bayer formats. Wait, didn't we fix this already? I've lost track. >> Ah, right, we were going to move this support into the IPUv3 driver, >> but in the meantime I think you had some patches to get around this. > > What I had was this patch for your v3. I never got to testing your > v4 because of the LP-11 problem. > > In v5, you've changed to propagate the ipu_cpmem_set_image() error > code to avoid the resulting corruption, but that leaves the other bits > of this patch unaddressed, along my "media: imx: smfc: add support > for bayer formats" patch. > > Your driver basically has no support for bayer formats. You added the patches to this driver that adds the bayer support, I don't think there is anything more required of the driver at this point to support bayer, the remaining work needs to happen in the IPUv3 driver. I'll see if I have time to write that patch to IPUv3, but it's simple, in fact what you wrote below can be translate directly into ipu_cpmem_set_image(). There's a few other places bayer needs to be treated in IPUv3, but it should be obvious by grepping for the reference to pixel formats. Steve > > diff --git a/drivers/staging/media/imx/imx-smfc.c b/drivers/staging/media/imx/imx-smfc.c > index 313732201a52..4351c0365cf4 100644 > --- a/drivers/staging/media/imx/imx-smfc.c > +++ b/drivers/staging/media/imx/imx-smfc.c > @@ -234,11 +234,6 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv) > buf1 = imx_media_dma_buf_get_next_queued(priv->out_ring); > priv->next = buf1; > > - image.phys0 = buf0->phys; > - image.phys1 = buf1->phys; > - ipu_cpmem_set_image(priv->smfc_ch, &image); > - > - > switch (image.pix.pixelformat) { > case V4L2_PIX_FMT_SBGGR8: > case V4L2_PIX_FMT_SGBRG8: > @@ -247,6 +242,10 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv) > burst_size = 8; > passthrough = true; > passthrough_bits = 8; > + ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height); > + ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline); > + ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys); > + ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys); > break; > > case V4L2_PIX_FMT_SBGGR16: > @@ -256,9 +255,17 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv) > burst_size = 4; > passthrough = true; > passthrough_bits = 16; > + ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height); > + ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline); > + ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys); > + ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys); > break; > > default: > + image.phys0 = buf0->phys; > + image.phys1 = buf1->phys; > + ipu_cpmem_set_image(priv->smfc_ch, &image); > + > burst_size = (outfmt->width & 0xf) ? 8 : 16; > > /* >