From: Steve Longerbeam <slongerbeam@gmail.com> To: Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org Cc: Mauro Carvalho Chehab <mchehab@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, "open list:DRM DRIVERS FOR FREESCALE IMX" <dri-devel@lists.freedesktop.org>, open list <linux-kernel@vger.kernel.org>, "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>, "open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org> Subject: Re: [PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types Date: Mon, 8 Oct 2018 14:59:30 -0700 [thread overview] Message-ID: <e2ae98c2-a6a3-19b9-bd85-664e5f80c4ab@gmail.com> (raw) In-Reply-To: <1538732679.3545.5.camel@pengutronix.de> Hi Philipp, On 10/05/2018 02:44 AM, Philipp Zabel wrote: > Hi Steve, > > On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote: > > >> + >> + /* framelines for NTSC / PAL */ >> + height = (std & V4L2_STD_525_60) ? 525 : 625; > I think this is a bit convoluted. Instead of initializing std, then > possibly changing it, and then comparing to the inital value, and then > checking it again to determine the new height, why not just: > > if (width == 720 && height == 480) { > std = V4L2_STD_NTSC; > height = 525; > } else if (width == 720 && height == 576) { > std = V4L2_STD_PAL; > height = 625; > } else { > dev_err(csi->ipu->dev, > "Unsupported interlaced video mode\n"); > ret = -EINVAL; > goto out_unlock; > } > > ? Yes that was a bit convoluted, fixed. > >> >> /* >> * if cycles is set, we need to handle this over multiple cycles as >> * generic/bayer data >> */ >> - if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) { >> - if_fmt.width *= incc->cycles; > If the input format width passed to ipu_csi_init_interface is not > multiplied by the number of cycles per pixel anymore, width in the > CSI_SENS_FRM_SIZE register will be set to the unmultiplied value from > infmt. > This breaks 779680e2e793 ("media: imx: add support for RGB565_2X8 on > parallel bus"). Oops, that was a mistake, thanks for catching, fixed. Steve
WARNING: multiple messages have this Message-ID (diff)
From: Steve Longerbeam <slongerbeam@gmail.com> To: Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org Cc: Mauro Carvalho Chehab <mchehab@kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>, "open list:DRM DRIVERS FOR FREESCALE IMX" <dri-devel@lists.freedesktop.org>, open list <linux-kernel@vger.kernel.org>, "open list:STAGING SUBSYSTEM" <devel@driverdev.osuosl.org>, "open list:FRAMEBUFFER LAYER" <linux-fbdev@vger.kernel.org> Subject: Re: [PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types Date: Mon, 08 Oct 2018 21:59:30 +0000 [thread overview] Message-ID: <e2ae98c2-a6a3-19b9-bd85-664e5f80c4ab@gmail.com> (raw) In-Reply-To: <1538732679.3545.5.camel@pengutronix.de> Hi Philipp, On 10/05/2018 02:44 AM, Philipp Zabel wrote: > Hi Steve, > > On Thu, 2018-10-04 at 11:53 -0700, Steve Longerbeam wrote: > > >> + >> + /* framelines for NTSC / PAL */ >> + height = (std & V4L2_STD_525_60) ? 525 : 625; > I think this is a bit convoluted. Instead of initializing std, then > possibly changing it, and then comparing to the inital value, and then > checking it again to determine the new height, why not just: > > if (width = 720 && height = 480) { > std = V4L2_STD_NTSC; > height = 525; > } else if (width = 720 && height = 576) { > std = V4L2_STD_PAL; > height = 625; > } else { > dev_err(csi->ipu->dev, > "Unsupported interlaced video mode\n"); > ret = -EINVAL; > goto out_unlock; > } > > ? Yes that was a bit convoluted, fixed. > >> >> /* >> * if cycles is set, we need to handle this over multiple cycles as >> * generic/bayer data >> */ >> - if (is_parallel_bus(&priv->upstream_ep) && incc->cycles) { >> - if_fmt.width *= incc->cycles; > If the input format width passed to ipu_csi_init_interface is not > multiplied by the number of cycles per pixel anymore, width in the > CSI_SENS_FRM_SIZE register will be set to the unmultiplied value from > infmt. > This breaks 779680e2e793 ("media: imx: add support for RGB565_2X8 on > parallel bus"). Oops, that was a mistake, thanks for catching, fixed. Steve
next prev parent reply other threads:[~2018-10-08 21:59 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-10-04 18:53 [PATCH v4 00/11] imx-media: Fixes for interlaced capture Steve Longerbeam 2018-10-04 18:53 ` [PATCH v4 01/11] media: videodev2.h: Add more field helper macros Steve Longerbeam 2018-10-04 18:53 ` [PATCH v4 02/11] gpu: ipu-csi: Swap fields according to input/output field types Steve Longerbeam 2018-10-04 18:53 ` Steve Longerbeam 2018-10-04 18:53 ` Steve Longerbeam 2018-10-05 9:44 ` Philipp Zabel 2018-10-05 9:44 ` Philipp Zabel 2018-10-05 9:44 ` Philipp Zabel 2018-10-08 21:59 ` Steve Longerbeam [this message] 2018-10-08 21:59 ` Steve Longerbeam 2018-10-08 21:59 ` Steve Longerbeam 2018-10-04 18:53 ` [PATCH v4 03/11] gpu: ipu-v3: Add planar support to interlaced scan Steve Longerbeam 2018-10-04 18:53 ` Steve Longerbeam 2018-10-04 18:53 ` Steve Longerbeam 2018-10-05 9:48 ` Philipp Zabel 2018-10-05 9:48 ` Philipp Zabel 2018-10-05 9:48 ` Philipp Zabel 2018-10-05 9:48 ` Philipp Zabel 2018-10-09 0:09 ` Steve Longerbeam 2018-10-09 0:09 ` Steve Longerbeam 2018-10-09 0:09 ` Steve Longerbeam 2018-10-04 18:53 ` [PATCH v4 04/11] media: imx: Fix field negotiation Steve Longerbeam 2018-10-05 10:17 ` Philipp Zabel 2018-10-04 18:53 ` [PATCH v4 05/11] media: imx-csi: Double crop height for alternate fields at sink Steve Longerbeam 2018-10-05 10:18 ` Philipp Zabel 2018-10-04 18:53 ` [PATCH v4 06/11] media: imx: interweave and odd-chroma-row skip are incompatible Steve Longerbeam 2018-10-05 10:20 ` Philipp Zabel 2018-10-04 18:53 ` [PATCH v4 07/11] media: imx-csi: Allow skipping odd chroma rows for YVU420 Steve Longerbeam 2018-10-05 10:20 ` Philipp Zabel 2018-10-04 18:53 ` [PATCH v4 08/11] media: imx: vdic: rely on VDIC for correct field order Steve Longerbeam 2018-10-04 18:53 ` [PATCH v4 09/11] media: imx-csi: Move crop/compose reset after filling default mbus fields Steve Longerbeam 2018-10-05 10:22 ` Philipp Zabel 2018-10-04 18:54 ` [PATCH v4 10/11] media: imx: Allow interweave with top/bottom lines swapped Steve Longerbeam 2018-10-05 10:43 ` Philipp Zabel 2018-10-09 1:07 ` Steve Longerbeam 2018-10-04 18:54 ` [PATCH v4 11/11] media: imx.rst: Update doc to reflect fixes to interlaced capture Steve Longerbeam 2018-10-05 10:52 ` Philipp Zabel 2018-10-09 1:09 ` Steve Longerbeam 2018-10-04 19:34 ` [PATCH v4 00/11] imx-media: Fixes for " Hans Verkuil 2018-10-04 20:16 ` Steve Longerbeam
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=e2ae98c2-a6a3-19b9-bd85-664e5f80c4ab@gmail.com \ --to=slongerbeam@gmail.com \ --cc=b.zolnierkie@samsung.com \ --cc=devel@driverdev.osuosl.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=gregkh@linuxfoundation.org \ --cc=linux-fbdev@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=p.zabel@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.