From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Steve Longerbeam <slongerbeam@gmail.com>
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 <steve_longerbeam@mentor.com>,
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
Subject: Re: [PATCH v5 00/39] i.MX Media Driver
Date: Mon, 13 Mar 2017 09:30:07 +0000 [thread overview]
Message-ID: <20170313093007.GD21222@n2100.armlinux.org.uk> (raw)
In-Reply-To: <20170313081625.GX21222@n2100.armlinux.org.uk>
On Mon, Mar 13, 2017 at 08:16:25AM +0000, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:
> > On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
> > >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.
>
> There is more work, because the way you've merged my changes to
> imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
> respect to the burst size.
>
> You always set it to 8 or 16 depending on the width:
>
> burst_size = (image.pix.width & 0xf) ? 8 : 16;
>
> ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
>
> and then you have my switch() statement which assigns burst_size.
> My _tested_ code removed the above, added the switch, which had
> a default case which reflected the above setting:
>
> default:
> burst_size = (outfmt->width & 0xf) ? 8 : 16;
>
> and then went on to set the burst size _after_ the switch statement:
>
> ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);
>
> The effect is unchanged for non-bayer formats. For bayer formats, the
> burst size is determined by the bayer data size.
>
> So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
> above is still required.
>
> I'm not convinced that fixing ipu_cpmem_set_image() is even the best
> solution - it's not as trivial as it looks on the surface:
>
> ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
> ipu_cpmem_set_stride(ch, pix->bytesperline);
>
> this is fine, it doesn't depend on the format. However, the next line:
>
> ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));
>
> does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
> isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
> DRM knows nothing about bayer formats, there aren't fourcc codes in
> DRM for it. The result is that v4l2_pix_fmt_to_drm_fourcc() returns
> -EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().
>
> ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
> it's a bug that this is not checked and propagated. If it is checked and
> propagated, then we need this to support bayer formats, and I don't see
> DRM people wanting bayer format fourcc codes added without there being
> a real DRM driver wanting to use them.
>
> Then there's the business of calculating the top-left offset of the image,
> which for bayer always needs to be an even number of pixels - as this
> function takes the top-left offset, it ought to respect it, but if it
> doesn't meet this criteria, what should it do? csi_idmac_setup_channel()
> always sets them to zero, but that's not really something that
> ipu_cpmem_set_image() should assume.
For the time being, I've restored the functionality along the same lines
as I originally had. This seems to get me working capture, but might
break non-bayer passthrough mode:
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index fc0036aa84d0..df336971a009 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -314,14 +314,6 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
image.phys0 = phys[0];
image.phys1 = phys[1];
- ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
- if (ret)
- return ret;
-
- burst_size = (image.pix.width & 0xf) ? 8 : 16;
-
- ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
-
/*
* Check for conditions that require the IPU to handle the
* data internally as generic data, aka passthrough mode:
@@ -346,15 +338,29 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
passthrough_bits = 16;
break;
default:
+ burst_size = (image.pix.width & 0xf) ? 8 : 16;
passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
sensor_ep->bus.parallel.bus_width >= 16);
passthrough_bits = 16;
break;
}
- if (passthrough)
+ if (passthrough) {
+ ipu_cpmem_set_resolution(priv->idmac_ch, image.rect.width,
+ image.rect.height);
+ ipu_cpmem_set_stride(priv->idmac_ch, image.pix.bytesperline);
+ ipu_cpmem_set_buffer(priv->idmac_ch, 0, image.phys0);
+ ipu_cpmem_set_buffer(priv->idmac_ch, 1, image.phys1);
+ ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
ipu_cpmem_set_format_passthrough(priv->idmac_ch,
passthrough_bits);
+ } else {
+ ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
+ if (ret)
+ return ret;
+
+ ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
+ }
/*
* Set the channel for the direct CSI-->memory via SMFC
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2017-03-13 9:31 UTC|newest]
Thread overview: 175+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-10 4:52 [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Steve Longerbeam
2017-03-16 21:21 ` Rob Herring
2017-03-10 4:52 ` [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver Steve Longerbeam
2017-03-20 15:02 ` Rob Herring
2017-03-10 4:52 ` [PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640 Steve Longerbeam
2017-03-20 15:03 ` Rob Herring
2017-03-10 4:52 ` [PATCH v5 04/39] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 05/39] ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their connections Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 06/39] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Steve Longerbeam
2017-03-10 18:59 ` Troy Kisky
2017-03-10 19:17 ` Fabio Estevam
2017-03-10 21:57 ` Pavel Machek
2017-03-10 22:05 ` Fabio Estevam
2017-03-15 18:49 ` Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 08/39] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 09/39] ARM: dts: imx6-sabresd: " Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 10/39] ARM: dts: imx6-sabreauto: create i2cmux for i2c3 Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 11/39] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 12/39] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 13/39] ARM: dts: imx6-sabreauto: add the ADV7180 video decoder Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 14/39] add mux and video interface bridge entity functions Steve Longerbeam
2017-03-10 4:52 ` [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam
2017-03-10 12:03 ` Hans Verkuil
2017-03-10 18:37 ` Steve Longerbeam
2017-03-10 23:30 ` Pavel Machek
2017-03-10 23:42 ` Steve Longerbeam
2017-03-11 11:39 ` Hans Verkuil
2017-03-11 18:14 ` Steve Longerbeam
2017-03-11 18:51 ` Russell King - ARM Linux
2017-03-11 18:58 ` Steve Longerbeam
2017-03-11 19:00 ` Steve Longerbeam
2017-03-13 10:02 ` Hans Verkuil
2017-03-13 10:45 ` Russell King - ARM Linux
2017-03-13 10:53 ` Hans Verkuil
2017-03-13 17:06 ` Steve Longerbeam
2017-03-13 17:10 ` Hans Verkuil
2017-03-13 21:47 ` Steve Longerbeam
2017-03-14 16:21 ` Nicolas Dufresne
2017-03-14 16:43 ` Steve Longerbeam
2017-03-16 22:15 ` Sakari Ailus
2017-03-14 16:47 ` Russell King - ARM Linux
2017-03-14 16:50 ` Steve Longerbeam
2017-03-14 18:26 ` Pavel Machek
2017-03-10 4:52 ` [PATCH v5 16/39] [media] v4l2: add a new-frame before end-of-frame event Steve Longerbeam
2017-03-10 12:07 ` Hans Verkuil
2017-03-10 4:52 ` [PATCH v5 17/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam
2017-03-10 11:45 ` Hans Verkuil
2017-03-10 4:52 ` [PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval Steve Longerbeam
2017-03-11 13:41 ` Sakari Ailus
2017-03-11 20:31 ` Steve Longerbeam
2017-03-16 22:17 ` Sakari Ailus
2017-03-10 4:52 ` [PATCH v5 19/39] [media] add Omnivision OV5640 sensor driver Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 20/39] platform: add video-multiplexer subdevice driver Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Steve Longerbeam
2017-03-11 13:49 ` Sakari Ailus
2017-03-11 18:20 ` Steve Longerbeam
2017-03-13 9:55 ` Hans Verkuil
2017-03-10 4:53 ` [PATCH v5 22/39] media: Add userspace header file for i.MX Steve Longerbeam
2017-03-10 11:49 ` Hans Verkuil
2017-03-10 23:32 ` Pavel Machek
2017-03-10 4:53 ` [PATCH v5 23/39] media: Add i.MX media core driver Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 24/39] media: imx: Add Capture Device Interface Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 25/39] media: imx: Add CSI subdev driver Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 26/39] media: imx: Add VDIC " Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 27/39] media: imx: Add IC subdev drivers Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 28/39] media: imx: Add MIPI CSI-2 Receiver subdev driver Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 29/39] ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 30/39] media: imx: add support for bayer formats Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 31/39] media: imx: csi: " Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 32/39] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 33/39] media: imx: mipi-csi2: enable setting and getting of frame rates Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 34/39] media: imx: csi: add __csi_get_fmt Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 35/39] media: imx: csi/fim: add support for frame intervals Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 36/39] media: imx: redo pixel format enumeration and negotiation Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 37/39] media: imx: csi: add frame skipping support Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt Steve Longerbeam
2017-03-19 15:22 ` Russell King - ARM Linux
2017-03-19 19:08 ` Steve Longerbeam
2017-03-20 11:55 ` Philipp Zabel
2017-03-20 12:08 ` Russell King - ARM Linux
2017-03-20 14:00 ` Philipp Zabel
2017-03-20 14:17 ` Russell King - ARM Linux
2017-03-20 17:16 ` Russell King - ARM Linux
2017-03-20 17:23 ` Philipp Zabel
2017-03-20 20:47 ` Russell King - ARM Linux
2017-03-21 4:03 ` Steve Longerbeam
2017-03-21 11:27 ` Russell King - ARM Linux
2017-03-21 23:56 ` Steve Longerbeam
2017-03-21 23:33 ` Steve Longerbeam
2017-03-20 17:40 ` Philipp Zabel
2017-03-20 17:59 ` Russell King - ARM Linux
2017-03-20 19:48 ` Steve Longerbeam
2017-03-10 4:53 ` [PATCH v5 39/39] media: imx: propagate sink pad formats to source pads Steve Longerbeam
2017-03-10 20:13 ` [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux
2017-03-10 23:20 ` Steve Longerbeam
2017-03-12 17:47 ` Russell King - ARM Linux
2017-03-12 0:30 ` Steve Longerbeam
2017-03-12 19:57 ` Russell King - ARM Linux
2017-03-12 20:05 ` Steve Longerbeam
2017-03-12 20:22 ` Russell King - ARM Linux
2017-03-13 4:26 ` Steve Longerbeam
2017-03-13 8:16 ` Russell King - ARM Linux
2017-03-13 9:30 ` Russell King - ARM Linux [this message]
2017-03-13 23:39 ` Steve Longerbeam
2017-03-13 23:37 ` Steve Longerbeam
2017-03-12 17:51 ` Russell King - ARM Linux
2017-03-12 19:21 ` Steve Longerbeam
2017-03-12 19:29 ` Russell King - ARM Linux
2017-03-12 19:44 ` Steve Longerbeam
2017-03-12 20:16 ` Steve Longerbeam
2017-03-12 20:36 ` Steve Longerbeam
2017-03-12 20:39 ` Steve Longerbeam
2017-03-12 20:40 ` Russell King - ARM Linux
2017-03-12 21:09 ` Russell King - ARM Linux
2017-03-14 17:29 ` Steve Longerbeam
2017-03-18 20:02 ` Steve Longerbeam
2017-03-12 19:47 ` Russell King - ARM Linux
2017-03-12 20:00 ` Steve Longerbeam
2017-03-12 20:59 ` Mauro Carvalho Chehab
2017-03-12 21:13 ` Russell King - ARM Linux
2017-03-12 22:10 ` Mauro Carvalho Chehab
2017-03-14 17:02 ` Steve Longerbeam
2017-03-18 19:22 ` Russell King - ARM Linux
2017-03-18 19:58 ` Steve Longerbeam
2017-03-18 20:43 ` Russell King - ARM Linux
2017-03-19 0:41 ` Nicolas Dufresne
2017-03-19 0:54 ` Russell King - ARM Linux
2017-03-19 14:33 ` Nicolas Dufresne
2017-03-19 14:51 ` Russell King - ARM Linux
2017-03-19 9:55 ` Russell King - ARM Linux
2017-03-19 14:45 ` Nicolas Dufresne
2017-03-19 13:57 ` Vladimir Zapolskiy
2017-03-19 14:21 ` Russell King - ARM Linux
2017-03-19 14:22 ` Russell King - ARM Linux
2017-03-19 15:00 ` Vladimir Zapolskiy
2017-03-19 15:09 ` Russell King - ARM Linux
2017-03-19 14:47 ` Nicolas Dufresne
2017-03-19 10:38 ` Russell King - ARM Linux
2017-03-19 10:48 ` [PATCH 1/4] media: imx-media-csi: fix v4l2-compliance check Russell King
2017-03-19 22:00 ` Steve Longerbeam
2017-03-19 10:48 ` [PATCH 2/4] media: imx: allow bayer pixel formats to be looked up Russell King
2017-03-19 22:02 ` Steve Longerbeam
2017-03-19 10:49 ` [PATCH 3/4] media: imx-csi: add frame size/interval enumeration Russell King
2017-03-19 22:18 ` Steve Longerbeam
2017-03-21 23:46 ` Steve Longerbeam
2017-03-19 10:49 ` [PATCH 4/4] media: imx-media-capture: add frame sizes/interval enumeration Russell King
2017-03-19 22:21 ` Steve Longerbeam
2017-03-19 22:39 ` Russell King - ARM Linux
2017-03-20 8:55 ` Philippe De Muyter
2017-03-20 9:05 ` Russell King - ARM Linux
2017-03-20 9:23 ` Philippe De Muyter
2017-03-20 10:41 ` Russell King - ARM Linux
2017-03-19 17:54 ` [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam
2017-03-19 18:04 ` Russell King - ARM Linux
2017-03-20 13:01 ` Hans Verkuil
2017-03-20 13:29 ` Russell King - ARM Linux
2017-03-20 13:57 ` Hans Verkuil
2017-03-20 14:11 ` Russell King - ARM Linux
2017-03-20 15:57 ` Hans Verkuil
2017-03-21 10:42 ` Niklas Söderlund
2017-03-21 10:59 ` Hans Verkuil
2017-03-21 11:36 ` Russell King - ARM Linux
2017-03-22 18:10 ` Nicolas Dufresne
2017-03-19 12:14 ` Russell King - ARM Linux
2017-03-19 18:37 ` Steve Longerbeam
2017-03-19 18:51 ` Russell King - ARM Linux
2017-03-19 18:56 ` Steve Longerbeam
2017-03-20 12:49 ` Hans Verkuil
2017-03-20 13:20 ` Philipp Zabel
2017-03-20 15:43 ` Russell King - ARM Linux
2017-03-20 16:29 ` Philipp Zabel
2017-03-20 16:35 ` Russell King - ARM Linux
2017-03-20 13:15 ` Philipp Zabel
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=20170313093007.GD21222@n2100.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew-ct.chen@mediatek.com \
--cc=arnd@arndb.de \
--cc=bparrot@ti.com \
--cc=devel@driverdev.osuosl.org \
--cc=devicetree@vger.kernel.org \
--cc=fabio.estevam@nxp.com \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=horms+renesas@verge.net.au \
--cc=hverkuil@xs4all.nl \
--cc=jean-christophe.trotin@st.com \
--cc=kernel@pengutronix.de \
--cc=laurent.pinchart+renesas@ideasonboard.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=markus.heiser@darmarIT.de \
--cc=mchehab@kernel.org \
--cc=minghsiu.tsai@mediatek.com \
--cc=nick@shmanahar.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=p.zabel@pengutronix.de \
--cc=pavel@ucw.cz \
--cc=robert.jarzmik@free.fr \
--cc=robh+dt@kernel.org \
--cc=sakari.ailus@linux.intel.com \
--cc=shawnguo@kernel.org \
--cc=shuah@kernel.org \
--cc=slongerbeam@gmail.com \
--cc=songjun.wu@microchip.com \
--cc=steve_longerbeam@mentor.com \
--cc=sudipm.mukherjee@gmail.com \
--cc=tiffany.lin@mediatek.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).