Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1596626 > unrolled thread
| Started by | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| First post | 2017-03-10 06:00 +0100 |
| Last post | 2017-03-12 23:20 +0100 |
| Articles | 20 on this page of 87 — 9 participants |
Back to article view | Back to linux.kernel
[PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 26/39] media: imx: Add VDIC subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 30/39] media: imx: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 35/39] media: imx: csi/fim: add support for frame intervals Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 20/39] platform: add video-multiplexer subdevice driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 29/39] ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 09/39] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 10/39] ARM: dts: imx6-sabreauto: create i2cmux for i2c3 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 08/39] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 24/39] media: imx: Add Capture Device Interface Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 32/39] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 33/39] media: imx: mipi-csi2: enable setting and getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 25/39] media: imx: Add CSI subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 05/39] ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their connections Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 11/39] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-11 14:50 +0100
Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 19:30 +0100
Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 11:00 +0100
[PATCH v5 37/39] media: imx: csi: add frame skipping support Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 22/39] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
Re: [PATCH v5 22/39] media: Add userspace header file for i.MX Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 12:50 +0100
Re: [PATCH v5 22/39] media: Add userspace header file for i.MX Pavel Machek <pavel@ucw.cz> - 2017-03-11 00:40 +0100
[PATCH v5 36/39] media: imx: redo pixel format enumeration and negotiation Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 13:10 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 19:40 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Pavel Machek <pavel@ucw.cz> - 2017-03-11 00:40 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 00:50 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-11 12:50 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 19:20 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 20:00 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:00 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:10 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 11:10 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 11:50 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 12:00 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 18:10 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 18:20 +0100
Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 22:50 +0100
[PATCH v5 31/39] media: imx: csi: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
[PATCH v5 14/39] add mux and video interface bridge entity functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
[PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Troy Kisky <troy.kisky@boundarydevices.com> - 2017-03-10 20:00 +0100
Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Fabio Estevam <festevam@gmail.com> - 2017-03-10 20:20 +0100
Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Pavel Machek <pavel@ucw.cz> - 2017-03-10 23:00 +0100
Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Fabio Estevam <festevam@gmail.com> - 2017-03-10 23:10 +0100
[PATCH v5 17/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
Re: [PATCH v5 17/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 12:50 +0100
[PATCH v5 12/39] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
[PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
Re: [PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-11 14:50 +0100
Re: [PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 21:40 +0100
[PATCH v5 04/39] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
[PATCH v5 16/39] [media] v4l2: add a new-frame before end-of-frame event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
Re: [PATCH v5 16/39] [media] v4l2: add a new-frame before end-of-frame event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 13:10 +0100
[PATCH v5 06/39] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
[PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 21:20 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 00:30 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 18:50 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 01:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:00 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:10 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:30 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 05:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 09:20 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 10:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-14 00:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-03-14 00:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 19:00 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 20:30 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 20:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 20:50 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:20 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:40 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:50 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 22:20 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 20:50 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:10 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 22:00 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 22:20 +0100
Re: [PATCH v5 00/39] i.MX Media Driver Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 23:20 +0100
Page 2 of 5 — ← Prev page 1 [2] 3 4 5 Next page →
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-13 11:00 +0100 |
| Subject | Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file |
| Message-ID | <tkuVH-74H-1@gated-at.bofh.it> |
| In reply to | #1598403 |
On 03/11/2017 07:20 PM, Steve Longerbeam wrote: > > > On 03/11/2017 05:49 AM, Sakari Ailus wrote: >> Hi Steve, >> >> On Thu, Mar 09, 2017 at 08:53:01PM -0800, Steve Longerbeam wrote: >>> Add an empty UAPI Kbuild file for media UAPI headers. >>> >>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com> >> >> The existing V4L2 UAPI headers are under include/uapi/linux. Could you use >> that directory instead? >> >> I actually wouldn't really object doing this but it should have been done in >> 2002 or so when the first V4L2 header was added. Now the benefit is >> questionable. >> > > Agreed, I think the current headers should be moved to uapi/media > eventually, but for now I'll go ahead and put under uapi/linux. No, while in staging it shouldn't be exported. Put it in include/linux and move it to uapi when this driver is mainlined. I don't think we can move headers from uapi/linux to uapi/media, I'm sure it's too late for that. Regards, Hans
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-10 06:00 +0100 |
| Subject | [PATCH v5 37/39] media: imx: csi: add frame skipping support |
| Message-ID | <tjkOL-708-57@gated-at.bofh.it> |
| In reply to | #1596626 |
From: Philipp Zabel <p.zabel@pengutronix.de>
The CSI can skip any out of up to 6 input frames, allowing to reduce the
frame rate at the output pads by small fractions.
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-media-csi.c | 125 ++++++++++++++++++++++++++++--
1 file changed, 120 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index a726744..e5105ec 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -1,13 +1,15 @@
/*
* V4L2 Capture CSI Subdev for Freescale i.MX5/6 SOC
*
- * Copyright (c) 2014-2016 Mentor Graphics Inc.
+ * Copyright (c) 2014-2017 Mentor Graphics Inc.
+ * Copyright (C) 2017 Pengutronix, Philipp Zabel <kernel@pengutronix.de>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*/
+#include <linux/gcd.h>
#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -40,6 +42,18 @@
#define H_ALIGN 1 /* multiple of 2 lines */
#define S_ALIGN 1 /* multiple of 2 */
+/*
+ * struct csi_skip_desc - CSI frame skipping descriptor
+ * @keep - number of frames kept per max_ratio frames
+ * @max_ratio - width of skip_smfc, written to MAX_RATIO bitfield
+ * @skip_smfc - skip pattern written to the SKIP_SMFC bitfield
+ */
+struct csi_skip_desc {
+ u8 keep;
+ u8 max_ratio;
+ u8 skip_smfc;
+};
+
struct csi_priv {
struct device *dev;
struct ipu_soc *ipu;
@@ -65,6 +79,7 @@ struct csi_priv {
const struct imx_media_pixfmt *cc[CSI_NUM_PADS];
struct v4l2_fract frame_interval;
struct v4l2_rect crop;
+ const struct csi_skip_desc *skip[CSI_NUM_PADS - 1];
/* active vb2 buffers to send to video dev sink */
struct imx_media_buffer *active_vb2_buf[2];
@@ -517,10 +532,12 @@ static int csi_setup(struct csi_priv *priv)
struct v4l2_mbus_config sensor_mbus_cfg;
struct v4l2_of_endpoint *sensor_ep;
struct v4l2_mbus_framefmt if_fmt;
+ const struct csi_skip_desc *skip;
infmt = &priv->format_mbus[CSI_SINK_PAD];
outfmt = &priv->format_mbus[priv->active_output_pad];
sensor_ep = &priv->sensor->sensor_ep;
+ skip = priv->skip[priv->active_output_pad - 1];
/* compose mbus_config from sensor endpoint */
sensor_mbus_cfg.type = sensor_ep->bus_type;
@@ -545,6 +562,9 @@ static int csi_setup(struct csi_priv *priv)
ipu_csi_set_dest(priv->csi, priv->dest);
+ ipu_csi_set_skip_smfc(priv->csi, skip->skip_smfc, skip->max_ratio - 1,
+ 0);
+
ipu_csi_dump(priv->csi);
return 0;
@@ -608,6 +628,77 @@ static void csi_stop(struct csi_priv *priv)
ipu_csi_disable(priv->csi);
}
+static const struct csi_skip_desc csi_skip[12] = {
+ { 1, 1, 0x00 }, /* Keep all frames */
+ { 5, 6, 0x10 }, /* Skip every sixth frame */
+ { 4, 5, 0x08 }, /* Skip every fifth frame */
+ { 3, 4, 0x04 }, /* Skip every fourth frame */
+ { 2, 3, 0x02 }, /* Skip every third frame */
+ { 3, 5, 0x0a }, /* Skip frames 1 and 3 of every 5 */
+ { 1, 2, 0x01 }, /* Skip every second frame */
+ { 2, 5, 0x0b }, /* Keep frames 1 and 4 of every 5 */
+ { 1, 3, 0x03 }, /* Keep one in three frames */
+ { 1, 4, 0x07 }, /* Keep one in four frames */
+ { 1, 5, 0x0f }, /* Keep one in five frames */
+ { 1, 6, 0x1f }, /* Keep one in six frames */
+};
+
+static void csi_apply_skip_interval(const struct csi_skip_desc *skip,
+ struct v4l2_fract *interval)
+{
+ unsigned int div;
+
+ interval->numerator *= skip->max_ratio;
+ interval->denominator *= skip->keep;
+
+ /* Reduce fraction to lowest terms */
+ div = gcd(interval->numerator, interval->denominator);
+ if (div > 1) {
+ interval->numerator /= div;
+ interval->denominator /= div;
+ }
+}
+
+/*
+ * Find the skip pattern to produce the output frame interval closest to the
+ * requested one, for the given input frame interval. Updates the output frame
+ * interval to the exact value.
+ */
+static const struct csi_skip_desc *csi_find_best_skip(struct v4l2_fract *in,
+ struct v4l2_fract *out)
+{
+ const struct csi_skip_desc *skip = &csi_skip[0], *best_skip = skip;
+ u32 min_err = UINT_MAX;
+ u64 want_us;
+ int i;
+
+ /* Default to 1:1 ratio */
+ if (out->numerator == 0 || out->denominator == 0 ||
+ in->numerator == 0 || in->denominator == 0)
+ return best_skip;
+
+ want_us = div_u64((u64)USEC_PER_SEC * out->numerator, out->denominator);
+
+ /* Find the reduction closest to the requested time per frame */
+ for (i = 0; i < ARRAY_SIZE(csi_skip); i++, skip++) {
+ u64 tmp, err;
+
+ tmp = div_u64((u64)USEC_PER_SEC * in->numerator *
+ skip->max_ratio, in->denominator * skip->keep);
+
+ err = abs((s64)tmp - want_us);
+ if (err < min_err) {
+ min_err = err;
+ best_skip = skip;
+ }
+ }
+
+ *out = *in;
+ csi_apply_skip_interval(best_skip, out);
+
+ return best_skip;
+}
+
/*
* V4L2 subdev operations.
*/
@@ -617,8 +708,16 @@ static int csi_g_frame_interval(struct v4l2_subdev *sd,
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ if (fi->pad >= CSI_NUM_PADS)
+ return -EINVAL;
+
mutex_lock(&priv->lock);
+
fi->interval = priv->frame_interval;
+
+ if (fi->pad != CSI_SINK_PAD)
+ csi_apply_skip_interval(priv->skip[fi->pad - 1], &fi->interval);
+
mutex_unlock(&priv->lock);
return 0;
@@ -629,14 +728,27 @@ static int csi_s_frame_interval(struct v4l2_subdev *sd,
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ if (fi->pad >= CSI_NUM_PADS)
+ return -EINVAL;
+
mutex_lock(&priv->lock);
- /* Output pads mirror active input pad, no limits on input pads */
- if (fi->pad == CSI_SRC_PAD_IDMAC || fi->pad == CSI_SRC_PAD_DIRECT)
- fi->interval = priv->frame_interval;
+ /* No limits on input pad */
+ if (fi->pad == CSI_SINK_PAD) {
+ priv->frame_interval = fi->interval;
+
+ /* Reset frame skipping ratio to 1:1 */
+ priv->skip[0] = &csi_skip[0];
+ priv->skip[1] = &csi_skip[0];
- priv->frame_interval = fi->interval;
+ goto out;
+ }
+
+ /* Output pads depend on input interval, modified by frame skipping */
+ priv->skip[fi->pad - 1] = csi_find_best_skip(&priv->frame_interval,
+ &fi->interval);
+out:
mutex_unlock(&priv->lock);
return 0;
@@ -1256,6 +1368,9 @@ static int csi_registered(struct v4l2_subdev *sd)
&priv->cc[i]);
if (ret)
goto put_csi;
+ /* disable frame skipping */
+ if (i != CSI_SINK_PAD)
+ priv->skip[i - 1] = &csi_skip[0];
}
/* init default frame interval */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-10 06:00 +0100 |
| Subject | [PATCH v5 22/39] media: Add userspace header file for i.MX |
| Message-ID | <tjkOK-708-47@gated-at.bofh.it> |
| In reply to | #1596626 |
This adds a header file for use by userspace programs wanting to interact
with the i.MX media driver. It defines custom v4l2 controls for the
i.MX v4l2 subdevices.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
include/uapi/media/Kbuild | 1 +
include/uapi/media/imx.h | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
create mode 100644 include/uapi/media/imx.h
diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
index aafaa5a..fa78958 100644
--- a/include/uapi/media/Kbuild
+++ b/include/uapi/media/Kbuild
@@ -1 +1,2 @@
# UAPI Header export list
+header-y += imx.h
diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
new file mode 100644
index 0000000..f573de4
--- /dev/null
+++ b/include/uapi/media/imx.h
@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2014-2015 Mentor Graphics Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version
+ */
+
+#ifndef __UAPI_MEDIA_IMX_H__
+#define __UAPI_MEDIA_IMX_H__
+
+enum imx_ctrl_id {
+ V4L2_CID_IMX_FIM_ENABLE = (V4L2_CID_USER_IMX_BASE + 0),
+ V4L2_CID_IMX_FIM_NUM,
+ V4L2_CID_IMX_FIM_TOLERANCE_MIN,
+ V4L2_CID_IMX_FIM_TOLERANCE_MAX,
+ V4L2_CID_IMX_FIM_NUM_SKIP,
+};
+
+#endif
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-10 12:50 +0100 |
| Subject | Re: [PATCH v5 22/39] media: Add userspace header file for i.MX |
| Message-ID | <tjrdy-3oE-65@gated-at.bofh.it> |
| In reply to | #1596646 |
On 10/03/17 05:53, Steve Longerbeam wrote:
> This adds a header file for use by userspace programs wanting to interact
> with the i.MX media driver. It defines custom v4l2 controls for the
> i.MX v4l2 subdevices.
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
I would not export this while the imx driver is in staging.
Also, traditionally exported media headers are in linux, not media.
I would like to have a discussion about that before deciding where to
place this header.
For the record: I am not opposed to placing this in media.
Regards,
Hans
> ---
> include/uapi/media/Kbuild | 1 +
> include/uapi/media/imx.h | 21 +++++++++++++++++++++
> 2 files changed, 22 insertions(+)
> create mode 100644 include/uapi/media/imx.h
>
> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> index aafaa5a..fa78958 100644
> --- a/include/uapi/media/Kbuild
> +++ b/include/uapi/media/Kbuild
> @@ -1 +1,2 @@
> # UAPI Header export list
> +header-y += imx.h
> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> new file mode 100644
> index 0000000..f573de4
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version
> + */
> +
> +#ifndef __UAPI_MEDIA_IMX_H__
> +#define __UAPI_MEDIA_IMX_H__
> +
> +enum imx_ctrl_id {
> + V4L2_CID_IMX_FIM_ENABLE = (V4L2_CID_USER_IMX_BASE + 0),
> + V4L2_CID_IMX_FIM_NUM,
> + V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> + V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> + V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +
> +#endif
>
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-03-11 00:40 +0100 |
| Subject | Re: [PATCH v5 22/39] media: Add userspace header file for i.MX |
| Message-ID | <tjCiB-2B7-11@gated-at.bofh.it> |
| In reply to | #1596646 |
[Multipart message — attachments visible in raw view] — view raw
Hi!
> diff --git a/include/uapi/media/Kbuild b/include/uapi/media/Kbuild
> index aafaa5a..fa78958 100644
> --- a/include/uapi/media/Kbuild
> +++ b/include/uapi/media/Kbuild
> @@ -1 +1,2 @@
> # UAPI Header export list
> +header-y += imx.h
> diff --git a/include/uapi/media/imx.h b/include/uapi/media/imx.h
> new file mode 100644
> index 0000000..f573de4
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,21 @@
> +/*
> + * Copyright (c) 2014-2015 Mentor Graphics Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version
> + */
> +
> +#ifndef __UAPI_MEDIA_IMX_H__
> +#define __UAPI_MEDIA_IMX_H__
> +
> +enum imx_ctrl_id {
> + V4L2_CID_IMX_FIM_ENABLE = (V4L2_CID_USER_IMX_BASE + 0),
> + V4L2_CID_IMX_FIM_NUM,
> + V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> + V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> + V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +
Should this #include something so that if userland includes it, it
will not get compile error?
Should there be some documentation of userland API?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-10 06:00 +0100 |
| Subject | [PATCH v5 36/39] media: imx: redo pixel format enumeration and negotiation |
| Message-ID | <tjkOL-708-55@gated-at.bofh.it> |
| In reply to | #1596626 |
The previous API and negotiation of mbus codes and pixel formats
was broken, and has been completely redone.
The negotiation of media bus codes should be as follows:
CSI:
sink pad direct src pad IDMAC src pad
-------- ---------------- -------------
RGB (any) IPU RGB IPU RGB
YUV (any) IPU YUV IPU YUV
Bayer N/A must be same bayer code as sink
VDIC:
direct sink pad IDMAC sink pad direct src pad
--------------- -------------- --------------
IPU YUV only IPU YUV only IPU YUV only
PRP:
direct sink pad direct src pads
--------------- ---------------
IPU (any) same as sink code
PRP ENC/VF:
direct sink pad IDMAC src pads
--------------- --------------
IPU (any) IPU RGB or IPU YUV
Given the above, a new internal API is created:
enum codespace_sel {
CS_SEL_YUV = 0, /* find or enumerate only YUV codes */
CS_SEL_RGB, /* find or enumerate only RGB codes */
CS_SEL_ANY, /* find or enumerate both YUV and RGB codes */
};
/* Find and enumerate fourcc pixel formats */
const struct imx_media_pixfmt *
imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
/* Find and enumerate media bus codes */
const struct imx_media_pixfmt *
imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
bool allow_bayer);
int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
bool allow_bayer);
/* Find and enumerate IPU internal media bus codes */
const struct imx_media_pixfmt *
imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel);
int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel);
The tables have been split into separate tables for YUV and RGB formats
to support the implementation of the above.
The subdev's .enum_mbus_code() and .set_fmt() operations have
been rewritten using the above APIs.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/staging/media/imx/imx-ic-prp.c | 77 ++++--
drivers/staging/media/imx/imx-ic-prpencvf.c | 57 ++--
drivers/staging/media/imx/imx-media-capture.c | 85 ++++--
drivers/staging/media/imx/imx-media-csi.c | 108 +++++---
drivers/staging/media/imx/imx-media-utils.c | 371 +++++++++++++++++++-------
drivers/staging/media/imx/imx-media-vdic.c | 69 ++---
drivers/staging/media/imx/imx-media.h | 27 +-
7 files changed, 504 insertions(+), 290 deletions(-)
diff --git a/drivers/staging/media/imx/imx-ic-prp.c b/drivers/staging/media/imx/imx-ic-prp.c
index 1832915..83cd2b4 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -89,16 +89,6 @@ static void prp_stop(struct prp_priv *priv)
{
}
-static int prp_enum_mbus_code(struct v4l2_subdev *sd,
- struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_mbus_code_enum *code)
-{
- if (code->pad >= PRP_NUM_PADS)
- return -EINVAL;
-
- return imx_media_enum_ipu_format(NULL, &code->code, code->index, true);
-}
-
static struct v4l2_mbus_framefmt *
__prp_get_fmt(struct prp_priv *priv, struct v4l2_subdev_pad_config *cfg,
unsigned int pad, enum v4l2_subdev_format_whence which)
@@ -115,6 +105,38 @@ __prp_get_fmt(struct prp_priv *priv, struct v4l2_subdev_pad_config *cfg,
* V4L2 subdev operations.
*/
+static int prp_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ struct prp_priv *priv = sd_to_priv(sd);
+ struct v4l2_mbus_framefmt *infmt;
+ int ret = 0;
+
+ mutex_lock(&priv->lock);
+
+ switch (code->pad) {
+ case PRP_SINK_PAD:
+ ret = imx_media_enum_ipu_format(&code->code, code->index,
+ CS_SEL_ANY);
+ break;
+ case PRP_SRC_PAD_PRPENC:
+ case PRP_SRC_PAD_PRPVF:
+ if (code->index != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ infmt = __prp_get_fmt(priv, cfg, PRP_SINK_PAD, code->which);
+ code->code = infmt->code;
+ break;
+ default:
+ ret = -EINVAL;
+ }
+out:
+ mutex_unlock(&priv->lock);
+ return ret;
+}
+
static int prp_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat)
@@ -160,23 +182,28 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
goto out;
}
- cc = imx_media_find_ipu_format(0, sdformat->format.code, true);
- if (!cc) {
- imx_media_enum_ipu_format(NULL, &code, 0, true);
- cc = imx_media_find_ipu_format(0, code, true);
- sdformat->format.code = cc->codes[0];
- }
-
- v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W,
- W_ALIGN, &sdformat->format.height,
- MIN_H, MAX_H, H_ALIGN, S_ALIGN);
-
- /* Output pads mirror input pad */
- if (sdformat->pad == PRP_SRC_PAD_PRPENC ||
- sdformat->pad == PRP_SRC_PAD_PRPVF) {
+ switch (sdformat->pad) {
+ case PRP_SINK_PAD:
+ v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W,
+ W_ALIGN, &sdformat->format.height,
+ MIN_H, MAX_H, H_ALIGN, S_ALIGN);
+
+ cc = imx_media_find_ipu_format(sdformat->format.code,
+ CS_SEL_ANY);
+ if (!cc) {
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_ANY);
+ cc = imx_media_find_ipu_format(code, CS_SEL_ANY);
+ sdformat->format.code = cc->codes[0];
+ }
+ break;
+ case PRP_SRC_PAD_PRPENC:
+ case PRP_SRC_PAD_PRPVF:
+ /* Output pads mirror input pad */
infmt = __prp_get_fmt(priv, cfg, PRP_SINK_PAD,
sdformat->which);
+ cc = imx_media_find_ipu_format(infmt->code, CS_SEL_ANY);
sdformat->format = *infmt;
+ break;
}
if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY) {
@@ -402,7 +429,7 @@ static int prp_registered(struct v4l2_subdev *sd)
MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
/* set a default mbus format */
- imx_media_enum_ipu_format(NULL, &code, 0, true);
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
640, 480, code, V4L2_FIELD_NONE,
&priv->cc[i]);
diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c b/drivers/staging/media/imx/imx-ic-prpencvf.c
index bd89e9b..b42103c 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -702,20 +702,6 @@ static void prp_stop(struct prp_priv *priv)
prp_put_ipu_resources(priv);
}
-static int prp_enum_mbus_code(struct v4l2_subdev *sd,
- struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_mbus_code_enum *code)
-{
- if (code->pad >= PRPENCVF_NUM_PADS)
- return -EINVAL;
-
- if (code->pad == PRPENCVF_SRC_PAD)
- return imx_media_enum_format(NULL, &code->code, code->index,
- true, false);
-
- return imx_media_enum_ipu_format(NULL, &code->code, code->index, true);
-}
-
static struct v4l2_mbus_framefmt *
__prp_get_fmt(struct prp_priv *priv, struct v4l2_subdev_pad_config *cfg,
unsigned int pad, enum v4l2_subdev_format_whence which)
@@ -732,6 +718,16 @@ __prp_get_fmt(struct prp_priv *priv, struct v4l2_subdev_pad_config *cfg,
* V4L2 subdev operations.
*/
+static int prp_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ if (code->pad >= PRPENCVF_NUM_PADS)
+ return -EINVAL;
+
+ return imx_media_enum_ipu_format(&code->code, code->index, CS_SEL_ANY);
+}
+
static int prp_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat)
@@ -777,18 +773,17 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
goto out;
}
+ cc = imx_media_find_ipu_format(sdformat->format.code, CS_SEL_ANY);
+ if (!cc) {
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_ANY);
+ cc = imx_media_find_ipu_format(code, CS_SEL_ANY);
+ sdformat->format.code = cc->codes[0];
+ }
+
if (sdformat->pad == PRPENCVF_SRC_PAD) {
infmt = __prp_get_fmt(priv, cfg, PRPENCVF_SINK_PAD,
sdformat->which);
- cc = imx_media_find_format(0, sdformat->format.code,
- true, false);
- if (!cc) {
- imx_media_enum_format(NULL, &code, 0, true, false);
- cc = imx_media_find_format(0, code, true, false);
- sdformat->format.code = cc->codes[0];
- }
-
if (sdformat->format.field != V4L2_FIELD_NONE)
sdformat->format.field = infmt->field;
@@ -808,14 +803,6 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
infmt->height / 4, MAX_H_SRC,
H_ALIGN_SRC, S_ALIGN);
} else {
- cc = imx_media_find_ipu_format(0, sdformat->format.code,
- true);
- if (!cc) {
- imx_media_enum_ipu_format(NULL, &code, 0, true);
- cc = imx_media_find_ipu_format(0, code, true);
- sdformat->format.code = cc->codes[0];
- }
-
v4l_bound_align_image(&sdformat->format.width,
MIN_W_SINK, MAX_W_SINK, W_ALIGN_SINK,
&sdformat->format.height,
@@ -1098,15 +1085,11 @@ static int prp_registered(struct v4l2_subdev *sd)
priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
for (i = 0; i < PRPENCVF_NUM_PADS; i++) {
- if (i == PRPENCVF_SINK_PAD) {
- priv->pad[i].flags = MEDIA_PAD_FL_SINK;
- imx_media_enum_ipu_format(NULL, &code, 0, true);
- } else {
- priv->pad[i].flags = MEDIA_PAD_FL_SOURCE;
- code = 0;
- }
+ priv->pad[i].flags = (i == PRPENCVF_SINK_PAD) ?
+ MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
/* set a default mbus format */
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
640, 480, code, V4L2_FIELD_NONE,
&priv->cc[i]);
diff --git a/drivers/staging/media/imx/imx-media-capture.c b/drivers/staging/media/imx/imx-media-capture.c
index 704ed85..a757e05 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -85,12 +85,39 @@ static int vidioc_querycap(struct file *file, void *fh,
static int capture_enum_fmt_vid_cap(struct file *file, void *fh,
struct v4l2_fmtdesc *f)
{
+ struct capture_priv *priv = video_drvdata(file);
+ const struct imx_media_pixfmt *cc_src;
+ struct v4l2_subdev_format fmt_src;
u32 fourcc;
int ret;
- ret = imx_media_enum_format(&fourcc, NULL, f->index, true, true);
- if (ret)
+ fmt_src.pad = priv->src_sd_pad;
+ fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
+ ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
+ if (ret) {
+ v4l2_err(priv->src_sd, "failed to get src_sd format\n");
return ret;
+ }
+
+ cc_src = imx_media_find_ipu_format(fmt_src.format.code, CS_SEL_ANY);
+ if (!cc_src)
+ cc_src = imx_media_find_mbus_format(fmt_src.format.code,
+ CS_SEL_ANY, true);
+ if (!cc_src)
+ return -EINVAL;
+
+ if (cc_src->bayer) {
+ if (f->index != 0)
+ return -EINVAL;
+ fourcc = cc_src->fourcc;
+ } else {
+ u32 cs_sel = (cc_src->cs == IPUV3_COLORSPACE_YUV) ?
+ CS_SEL_YUV : CS_SEL_RGB;
+
+ ret = imx_media_enum_format(&fourcc, f->index, cs_sel);
+ if (ret)
+ return ret;
+ }
f->pixelformat = fourcc;
@@ -112,40 +139,40 @@ static int capture_try_fmt_vid_cap(struct file *file, void *fh,
{
struct capture_priv *priv = video_drvdata(file);
struct v4l2_subdev_format fmt_src;
- const struct imx_media_pixfmt *cc, *src_cc;
- u32 fourcc;
+ const struct imx_media_pixfmt *cc, *cc_src;
int ret;
- fourcc = f->fmt.pix.pixelformat;
- cc = imx_media_find_format(fourcc, 0, true, true);
- if (!cc) {
- imx_media_enum_format(&fourcc, NULL, 0, true, true);
- cc = imx_media_find_format(fourcc, 0, true, true);
- }
-
- /*
- * user frame dimensions are the same as src_sd's pad.
- */
fmt_src.pad = priv->src_sd_pad;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
ret = v4l2_subdev_call(priv->src_sd, pad, get_fmt, NULL, &fmt_src);
if (ret)
return ret;
- /*
- * but we can allow planar pixel formats if the src_sd's
- * pad configured a YUV format
- */
- src_cc = imx_media_find_format(0, fmt_src.format.code, true, false);
- if (src_cc->cs == IPUV3_COLORSPACE_YUV &&
- cc->cs == IPUV3_COLORSPACE_YUV) {
- imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix,
- &fmt_src.format, cc);
+ cc_src = imx_media_find_ipu_format(fmt_src.format.code, CS_SEL_ANY);
+ if (!cc_src)
+ cc_src = imx_media_find_mbus_format(fmt_src.format.code,
+ CS_SEL_ANY, true);
+ if (!cc_src)
+ return -EINVAL;
+
+ if (cc_src->bayer) {
+ cc = cc_src;
} else {
- imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix,
- &fmt_src.format, src_cc);
+ u32 fourcc, cs_sel;
+
+ cs_sel = (cc_src->cs == IPUV3_COLORSPACE_YUV) ?
+ CS_SEL_YUV : CS_SEL_RGB;
+ fourcc = f->fmt.pix.pixelformat;
+
+ cc = imx_media_find_format(fourcc, cs_sel);
+ if (!cc) {
+ imx_media_enum_format(&fourcc, 0, cs_sel);
+ cc = imx_media_find_format(fourcc, cs_sel);
+ }
}
+ imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, &fmt_src.format, cc);
+
return 0;
}
@@ -165,8 +192,8 @@ static int capture_s_fmt_vid_cap(struct file *file, void *fh,
return ret;
priv->vdev.fmt.fmt.pix = f->fmt.pix;
- priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat, 0,
- true, true);
+ priv->vdev.cc = imx_media_find_format(f->fmt.pix.pixelformat,
+ CS_SEL_ANY);
return 0;
}
@@ -574,8 +601,8 @@ int imx_media_capture_device_register(struct imx_media_video_dev *vdev)
vdev->fmt.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
imx_media_mbus_fmt_to_pix_fmt(&vdev->fmt.fmt.pix,
&fmt_src.format, NULL);
- vdev->cc = imx_media_find_format(0, fmt_src.format.code,
- true, false);
+ vdev->cc = imx_media_find_format(vdev->fmt.fmt.pix.pixelformat,
+ CS_SEL_ANY);
v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
video_device_node_name(vfd));
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index b556fa4..a726744 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -917,15 +917,44 @@ static int csi_enum_mbus_code(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_mbus_code_enum *code)
{
- if (code->pad >= CSI_NUM_PADS)
- return -EINVAL;
+ struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ const struct imx_media_pixfmt *incc;
+ struct v4l2_mbus_framefmt *infmt;
+ int ret = 0;
- if (code->pad == CSI_SRC_PAD_DIRECT)
- return imx_media_enum_ipu_format(NULL, &code->code,
- code->index, true);
+ mutex_lock(&priv->lock);
+
+ infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, code->which);
+ incc = imx_media_find_mbus_format(infmt->code, CS_SEL_ANY, true);
- return imx_media_enum_format(NULL, &code->code, code->index,
- true, false);
+ switch (code->pad) {
+ case CSI_SINK_PAD:
+ ret = imx_media_enum_mbus_format(&code->code, code->index,
+ CS_SEL_ANY, true);
+ break;
+ case CSI_SRC_PAD_DIRECT:
+ case CSI_SRC_PAD_IDMAC:
+ if (incc->bayer) {
+ if (code->index != 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ code->code = infmt->code;
+ } else {
+ u32 cs_sel = (incc->cs == IPUV3_COLORSPACE_YUV) ?
+ CS_SEL_YUV : CS_SEL_RGB;
+ ret = imx_media_enum_ipu_format(&code->code,
+ code->index,
+ cs_sel);
+ }
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+out:
+ mutex_unlock(&priv->lock);
+ return ret;
}
static int csi_get_fmt(struct v4l2_subdev *sd,
@@ -981,14 +1010,13 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
goto out;
}
- v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W,
- W_ALIGN, &sdformat->format.height,
- MIN_H, MAX_H, H_ALIGN, S_ALIGN);
-
switch (sdformat->pad) {
case CSI_SRC_PAD_DIRECT:
case CSI_SRC_PAD_IDMAC:
- infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sdformat->which);
+ infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD,
+ sdformat->which);
+ incc = imx_media_find_mbus_format(infmt->code,
+ CS_SEL_ANY, true);
if (sdformat->format.width < priv->crop.width * 3 / 4)
sdformat->format.width = priv->crop.width / 2;
@@ -1000,38 +1028,25 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
else
sdformat->format.height = priv->crop.height;
- if (sdformat->pad == CSI_SRC_PAD_IDMAC) {
- cc = imx_media_find_format(0, sdformat->format.code,
- true, false);
- if (!cc) {
- imx_media_enum_format(NULL, &code, 0,
- true, false);
- cc = imx_media_find_format(0, code,
- true, false);
- sdformat->format.code = cc->codes[0];
- }
-
- incc = priv->cc[CSI_SINK_PAD];
- if (cc->cs != incc->cs) {
- sdformat->format.code = infmt->code;
- cc = imx_media_find_format(
- 0, sdformat->format.code,
- true, false);
- }
-
- if (sdformat->format.field != V4L2_FIELD_NONE)
- sdformat->format.field = infmt->field;
+ if (incc->bayer) {
+ sdformat->format.code = infmt->code;
+ cc = incc;
} else {
- cc = imx_media_find_ipu_format(0, sdformat->format.code,
- true);
+ u32 cs_sel = (incc->cs == IPUV3_COLORSPACE_YUV) ?
+ CS_SEL_YUV : CS_SEL_RGB;
+
+ cc = imx_media_find_ipu_format(sdformat->format.code,
+ cs_sel);
if (!cc) {
- imx_media_enum_ipu_format(NULL, &code, 0, true);
- cc = imx_media_find_ipu_format(0, code, true);
+ imx_media_enum_ipu_format(&code, 0, cs_sel);
+ cc = imx_media_find_ipu_format(code, cs_sel);
sdformat->format.code = cc->codes[0];
}
+ }
+ if (sdformat->pad == CSI_SRC_PAD_DIRECT ||
+ sdformat->format.field != V4L2_FIELD_NONE)
sdformat->format.field = infmt->field;
- }
/*
* translate V4L2_FIELD_ALTERNATE to SEQ_TB or SEQ_BT
@@ -1044,6 +1059,9 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
}
break;
case CSI_SINK_PAD:
+ v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W,
+ W_ALIGN, &sdformat->format.height,
+ MIN_H, MAX_H, H_ALIGN, S_ALIGN);
crop.left = 0;
crop.top = 0;
crop.width = sdformat->format.width;
@@ -1053,11 +1071,13 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
if (ret)
goto out;
- cc = imx_media_find_format(0, sdformat->format.code,
- true, false);
+ cc = imx_media_find_mbus_format(sdformat->format.code,
+ CS_SEL_ANY, true);
if (!cc) {
- imx_media_enum_format(NULL, &code, 0, true, false);
- cc = imx_media_find_format(0, code, true, false);
+ imx_media_enum_mbus_format(&code, 0,
+ CS_SEL_ANY, false);
+ cc = imx_media_find_mbus_format(code,
+ CS_SEL_ANY, false);
sdformat->format.code = cc->codes[0];
}
break;
@@ -1227,8 +1247,8 @@ static int csi_registered(struct v4l2_subdev *sd)
MEDIA_PAD_FL_SINK : MEDIA_PAD_FL_SOURCE;
code = 0;
- if (i == CSI_SRC_PAD_DIRECT)
- imx_media_enum_ipu_format(NULL, &code, 0, true);
+ if (i != CSI_SINK_PAD)
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
/* set a default mbus format */
ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 24e3795..a648fd5 100644
--- a/drivers/staging/media/imx/imx-media-utils.c
+++ b/drivers/staging/media/imx/imx-media-utils.c
@@ -12,14 +12,13 @@
#include "imx-media.h"
/*
- * List of pixel formats for the subdevs. This must be a super-set of
- * the formats supported by the ipu image converter.
+ * List of supported pixel formats for the subdevs.
*
- * The non-mbus formats (planar and BGR) must all fall at the end of
- * this table, otherwise enum_fmt() at media pads will stop before
- * seeing all the supported mbus formats.
+ * In all of these tables, the non-mbus formats (with no
+ * mbus codes) must all fall at the end of the table.
*/
-static const struct imx_media_pixfmt imx_media_formats[] = {
+
+static const struct imx_media_pixfmt yuv_formats[] = {
{
.fourcc = V4L2_PIX_FMT_UYVY,
.codes = {
@@ -36,13 +35,45 @@ static const struct imx_media_pixfmt imx_media_formats[] = {
},
.cs = IPUV3_COLORSPACE_YUV,
.bpp = 16,
+ },
+ /***
+ * non-mbus YUV formats start here. NOTE! when adding non-mbus
+ * formats, NUM_NON_MBUS_YUV_FORMATS must be updated below.
+ ***/
+ {
+ .fourcc = V4L2_PIX_FMT_YUV420,
+ .cs = IPUV3_COLORSPACE_YUV,
+ .bpp = 12,
+ .planar = true,
}, {
- .fourcc = V4L2_PIX_FMT_YUV32,
- .codes = {MEDIA_BUS_FMT_AYUV8_1X32},
+ .fourcc = V4L2_PIX_FMT_YVU420,
.cs = IPUV3_COLORSPACE_YUV,
- .bpp = 32,
- .ipufmt = true,
+ .bpp = 12,
+ .planar = true,
}, {
+ .fourcc = V4L2_PIX_FMT_YUV422P,
+ .cs = IPUV3_COLORSPACE_YUV,
+ .bpp = 16,
+ .planar = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_NV12,
+ .cs = IPUV3_COLORSPACE_YUV,
+ .bpp = 12,
+ .planar = true,
+ }, {
+ .fourcc = V4L2_PIX_FMT_NV16,
+ .cs = IPUV3_COLORSPACE_YUV,
+ .bpp = 16,
+ .planar = true,
+ },
+};
+
+#define NUM_NON_MBUS_YUV_FORMATS 5
+#define NUM_YUV_FORMATS ARRAY_SIZE(yuv_formats)
+#define NUM_MBUS_YUV_FORMATS (NUM_YUV_FORMATS - NUM_NON_MBUS_YUV_FORMATS)
+
+static const struct imx_media_pixfmt rgb_formats[] = {
+ {
.fourcc = V4L2_PIX_FMT_RGB565,
.codes = {MEDIA_BUS_FMT_RGB565_2X8_LE},
.cs = IPUV3_COLORSPACE_RGB,
@@ -61,7 +92,9 @@ static const struct imx_media_pixfmt imx_media_formats[] = {
.cs = IPUV3_COLORSPACE_RGB,
.bpp = 32,
.ipufmt = true,
- }, {
+ },
+ /*** raw bayer formats start here ***/
+ {
.fourcc = V4L2_PIX_FMT_SBGGR8,
.codes = {MEDIA_BUS_FMT_SBGGR8_1X8},
.cs = IPUV3_COLORSPACE_RGB,
@@ -130,7 +163,10 @@ static const struct imx_media_pixfmt imx_media_formats[] = {
.bpp = 16,
.bayer = true,
},
- /*** non-mbus formats start here ***/
+ /***
+ * non-mbus RGB formats start here. NOTE! when adding non-mbus
+ * formats, NUM_NON_MBUS_RGB_FORMATS must be updated below.
+ ***/
{
.fourcc = V4L2_PIX_FMT_BGR24,
.cs = IPUV3_COLORSPACE_RGB,
@@ -139,135 +175,256 @@ static const struct imx_media_pixfmt imx_media_formats[] = {
.fourcc = V4L2_PIX_FMT_BGR32,
.cs = IPUV3_COLORSPACE_RGB,
.bpp = 32,
- }, {
- .fourcc = V4L2_PIX_FMT_YUV420,
- .cs = IPUV3_COLORSPACE_YUV,
- .bpp = 12,
- .planar = true,
- }, {
- .fourcc = V4L2_PIX_FMT_YVU420,
- .cs = IPUV3_COLORSPACE_YUV,
- .bpp = 12,
- .planar = true,
- }, {
- .fourcc = V4L2_PIX_FMT_YUV422P,
- .cs = IPUV3_COLORSPACE_YUV,
- .bpp = 16,
- .planar = true,
- }, {
- .fourcc = V4L2_PIX_FMT_NV12,
- .cs = IPUV3_COLORSPACE_YUV,
- .bpp = 12,
- .planar = true,
- }, {
- .fourcc = V4L2_PIX_FMT_NV16,
+ },
+};
+
+#define NUM_NON_MBUS_RGB_FORMATS 2
+#define NUM_RGB_FORMATS ARRAY_SIZE(rgb_formats)
+#define NUM_MBUS_RGB_FORMATS (NUM_RGB_FORMATS - NUM_NON_MBUS_RGB_FORMATS)
+
+static const struct imx_media_pixfmt ipu_yuv_formats[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_YUV32,
+ .codes = {MEDIA_BUS_FMT_AYUV8_1X32},
.cs = IPUV3_COLORSPACE_YUV,
- .bpp = 16,
- .planar = true,
+ .bpp = 32,
+ .ipufmt = true,
},
};
-static const u32 imx_media_ipu_internal_codes[] = {
- MEDIA_BUS_FMT_AYUV8_1X32, MEDIA_BUS_FMT_ARGB8888_1X32,
+#define NUM_IPU_YUV_FORMATS ARRAY_SIZE(ipu_yuv_formats)
+
+static const struct imx_media_pixfmt ipu_rgb_formats[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_RGB32,
+ .codes = {MEDIA_BUS_FMT_ARGB8888_1X32},
+ .cs = IPUV3_COLORSPACE_RGB,
+ .bpp = 32,
+ .ipufmt = true,
+ },
};
+#define NUM_IPU_RGB_FORMATS ARRAY_SIZE(ipu_rgb_formats)
+
static inline u32 pixfmt_to_colorspace(const struct imx_media_pixfmt *fmt)
{
return (fmt->cs == IPUV3_COLORSPACE_RGB) ?
V4L2_COLORSPACE_SRGB : V4L2_COLORSPACE_SMPTE170M;
}
-static const struct imx_media_pixfmt *find_format(u32 fourcc, u32 code,
- bool allow_rgb,
- bool allow_planar,
- bool ipu_fmt_only)
+static const struct imx_media_pixfmt *find_format(u32 fourcc,
+ u32 code,
+ enum codespace_sel cs_sel,
+ bool allow_non_mbus,
+ bool allow_bayer)
{
- const struct imx_media_pixfmt *fmt, *ret = NULL;
+ const struct imx_media_pixfmt *array, *fmt, *ret = NULL;
+ u32 array_size;
int i, j;
- for (i = 0; i < ARRAY_SIZE(imx_media_formats); i++) {
- fmt = &imx_media_formats[i];
+ switch (cs_sel) {
+ case CS_SEL_YUV:
+ array_size = NUM_YUV_FORMATS;
+ array = yuv_formats;
+ break;
+ case CS_SEL_RGB:
+ array_size = NUM_RGB_FORMATS;
+ array = rgb_formats;
+ break;
+ case CS_SEL_ANY:
+ array_size = NUM_YUV_FORMATS + NUM_RGB_FORMATS;
+ array = yuv_formats;
+ break;
+ default:
+ return NULL;
+ }
+
+ for (i = 0; i < array_size; i++) {
+ if (cs_sel == CS_SEL_ANY && i >= NUM_YUV_FORMATS)
+ fmt = &rgb_formats[i - NUM_YUV_FORMATS];
+ else
+ fmt = &array[i];
- if (ipu_fmt_only && !fmt->ipufmt)
+ if ((!allow_non_mbus && fmt->codes[0] == 0) ||
+ (!allow_bayer && fmt->bayer))
continue;
- if (fourcc && fmt->fourcc == fourcc &&
- (fmt->cs != IPUV3_COLORSPACE_RGB || allow_rgb) &&
- (!fmt->planar || allow_planar)) {
+ if (fourcc && fmt->fourcc == fourcc) {
ret = fmt;
goto out;
}
for (j = 0; code && fmt->codes[j]; j++) {
- if (fmt->codes[j] == code && !fmt->planar &&
- (fmt->cs != IPUV3_COLORSPACE_RGB || allow_rgb)) {
+ if (code == fmt->codes[j]) {
ret = fmt;
goto out;
}
}
}
+
out:
return ret;
}
-const struct imx_media_pixfmt *imx_media_find_format(u32 fourcc, u32 code,
- bool allow_rgb,
- bool allow_planar)
+static int enum_format(u32 *fourcc, u32 *code, u32 index,
+ enum codespace_sel cs_sel,
+ bool allow_non_mbus,
+ bool allow_bayer)
+{
+ const struct imx_media_pixfmt *fmt;
+ u32 mbus_yuv_sz = NUM_MBUS_YUV_FORMATS;
+ u32 mbus_rgb_sz = NUM_MBUS_RGB_FORMATS;
+ u32 yuv_sz = NUM_YUV_FORMATS;
+ u32 rgb_sz = NUM_RGB_FORMATS;
+
+ switch (cs_sel) {
+ case CS_SEL_YUV:
+ if (index >= yuv_sz ||
+ (!allow_non_mbus && index >= mbus_yuv_sz))
+ return -EINVAL;
+ fmt = &yuv_formats[index];
+ break;
+ case CS_SEL_RGB:
+ if (index >= rgb_sz ||
+ (!allow_non_mbus && index >= mbus_rgb_sz))
+ return -EINVAL;
+ fmt = &rgb_formats[index];
+ if (!allow_bayer && fmt->bayer)
+ return -EINVAL;
+ break;
+ case CS_SEL_ANY:
+ if (!allow_non_mbus) {
+ if (index >= mbus_yuv_sz) {
+ index -= mbus_yuv_sz;
+ if (index >= mbus_rgb_sz)
+ return -EINVAL;
+ fmt = &rgb_formats[index];
+ if (!allow_bayer && fmt->bayer)
+ return -EINVAL;
+ } else {
+ fmt = &yuv_formats[index];
+ }
+ } else {
+ if (index >= yuv_sz + rgb_sz)
+ return -EINVAL;
+ if (index >= yuv_sz) {
+ fmt = &rgb_formats[index - yuv_sz];
+ if (!allow_bayer && fmt->bayer)
+ return -EINVAL;
+ } else {
+ fmt = &yuv_formats[index];
+ }
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (fourcc)
+ *fourcc = fmt->fourcc;
+ if (code)
+ *code = fmt->codes[0];
+
+ return 0;
+}
+
+const struct imx_media_pixfmt *
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel)
{
- return find_format(fourcc, code, allow_rgb, allow_planar, false);
+ return find_format(fourcc, 0, cs_sel, true, false);
}
EXPORT_SYMBOL_GPL(imx_media_find_format);
-const struct imx_media_pixfmt *imx_media_find_ipu_format(u32 fourcc,
- u32 code,
- bool allow_rgb)
+int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel)
{
- return find_format(fourcc, code, allow_rgb, false, true);
+ return enum_format(fourcc, NULL, index, cs_sel, true, false);
}
-EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
+EXPORT_SYMBOL_GPL(imx_media_enum_format);
-int imx_media_enum_format(u32 *fourcc, u32 *code, u32 index,
- bool allow_rgb, bool allow_planar)
+const struct imx_media_pixfmt *
+imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
+ bool allow_bayer)
{
- const struct imx_media_pixfmt *fmt;
+ return find_format(0, code, cs_sel, false, allow_bayer);
+}
+EXPORT_SYMBOL_GPL(imx_media_find_mbus_format);
- if (index >= ARRAY_SIZE(imx_media_formats))
- return -EINVAL;
+int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
+ bool allow_bayer)
+{
+ return enum_format(NULL, code, index, cs_sel, false, allow_bayer);
+}
+EXPORT_SYMBOL_GPL(imx_media_enum_mbus_format);
- fmt = &imx_media_formats[index];
+const struct imx_media_pixfmt *
+imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel)
+{
+ const struct imx_media_pixfmt *array, *fmt, *ret = NULL;
+ u32 array_size;
+ int i, j;
- if ((fmt->cs == IPUV3_COLORSPACE_RGB && !allow_rgb) ||
- (fmt->planar && !allow_planar))
- return -EINVAL;
+ switch (cs_sel) {
+ case CS_SEL_YUV:
+ array_size = NUM_IPU_YUV_FORMATS;
+ array = ipu_yuv_formats;
+ break;
+ case CS_SEL_RGB:
+ array_size = NUM_IPU_RGB_FORMATS;
+ array = ipu_rgb_formats;
+ break;
+ case CS_SEL_ANY:
+ array_size = NUM_IPU_YUV_FORMATS + NUM_IPU_RGB_FORMATS;
+ array = ipu_yuv_formats;
+ break;
+ default:
+ return NULL;
+ }
- if (code)
- *code = fmt->codes[0];
- if (fourcc)
- *fourcc = fmt->fourcc;
+ for (i = 0; i < array_size; i++) {
+ if (cs_sel == CS_SEL_ANY && i >= NUM_IPU_YUV_FORMATS)
+ fmt = &ipu_rgb_formats[i - NUM_IPU_YUV_FORMATS];
+ else
+ fmt = &array[i];
- return 0;
+ for (j = 0; code && fmt->codes[j]; j++) {
+ if (code == fmt->codes[j]) {
+ ret = fmt;
+ goto out;
+ }
+ }
+ }
+
+out:
+ return ret;
}
-EXPORT_SYMBOL_GPL(imx_media_enum_format);
+EXPORT_SYMBOL_GPL(imx_media_find_ipu_format);
-int imx_media_enum_ipu_format(u32 *fourcc, u32 *code, u32 index,
- bool allow_rgb)
+int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel)
{
- const struct imx_media_pixfmt *fmt;
- u32 lcode;
-
- if (index >= ARRAY_SIZE(imx_media_ipu_internal_codes))
- return -EINVAL;
-
- lcode = imx_media_ipu_internal_codes[index];
-
- fmt = find_format(0, lcode, allow_rgb, false, true);
- if (!fmt)
+ switch (cs_sel) {
+ case CS_SEL_YUV:
+ if (index >= NUM_IPU_YUV_FORMATS)
+ return -EINVAL;
+ *code = ipu_yuv_formats[index].codes[0];
+ break;
+ case CS_SEL_RGB:
+ if (index >= NUM_IPU_RGB_FORMATS)
+ return -EINVAL;
+ *code = ipu_rgb_formats[index].codes[0];
+ break;
+ case CS_SEL_ANY:
+ if (index >= NUM_IPU_YUV_FORMATS + NUM_IPU_RGB_FORMATS)
+ return -EINVAL;
+ if (index >= NUM_IPU_YUV_FORMATS) {
+ index -= NUM_IPU_YUV_FORMATS;
+ *code = ipu_rgb_formats[index].codes[0];
+ } else {
+ *code = ipu_yuv_formats[index].codes[0];
+ }
+ break;
+ default:
return -EINVAL;
-
- if (code)
- *code = fmt->codes[0];
- if (fourcc)
- *fourcc = fmt->fourcc;
+ }
return 0;
}
@@ -283,10 +440,14 @@ int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
mbus->height = height;
mbus->field = field;
if (code == 0)
- imx_media_enum_format(NULL, &code, 0, true, false);
- lcc = imx_media_find_format(0, code, true, false);
- if (!lcc)
- return -EINVAL;
+ imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV, false);
+ lcc = imx_media_find_mbus_format(code, CS_SEL_ANY, false);
+ if (!lcc) {
+ lcc = imx_media_find_ipu_format(code, CS_SEL_ANY);
+ if (!lcc)
+ return -EINVAL;
+ }
+
mbus->code = code;
mbus->colorspace = pixfmt_to_colorspace(lcc);
@@ -304,11 +465,25 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix,
u32 stride;
if (!cc) {
- cc = imx_media_find_format(0, mbus->code, true, false);
+ cc = imx_media_find_ipu_format(mbus->code, CS_SEL_ANY);
+ if (!cc)
+ cc = imx_media_find_mbus_format(mbus->code, CS_SEL_ANY,
+ true);
if (!cc)
return -EINVAL;
}
+ /*
+ * TODO: the IPU currently does not support the AYUV32 format,
+ * so until it does convert to a supported YUV format.
+ */
+ if (cc->ipufmt && cc->cs == IPUV3_COLORSPACE_YUV) {
+ u32 code;
+
+ imx_media_enum_mbus_format(&code, 0, CS_SEL_YUV, false);
+ cc = imx_media_find_mbus_format(code, CS_SEL_YUV, false);
+ }
+
stride = cc->planar ? mbus->width : (mbus->width * cc->bpp) >> 3;
pix->width = mbus->width;
@@ -349,7 +524,7 @@ int imx_media_ipu_image_to_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
{
const struct imx_media_pixfmt *fmt;
- fmt = imx_media_find_format(image->pix.pixelformat, 0, true, false);
+ fmt = imx_media_find_format(image->pix.pixelformat, CS_SEL_ANY);
if (!fmt)
return -EINVAL;
diff --git a/drivers/staging/media/imx/imx-media-vdic.c b/drivers/staging/media/imx/imx-media-vdic.c
index 0e7c24c..070f9da 100644
--- a/drivers/staging/media/imx/imx-media-vdic.c
+++ b/drivers/staging/media/imx/imx-media-vdic.c
@@ -517,20 +517,6 @@ static int vdic_s_stream(struct v4l2_subdev *sd, int enable)
return ret;
}
-static int vdic_enum_mbus_code(struct v4l2_subdev *sd,
- struct v4l2_subdev_pad_config *cfg,
- struct v4l2_subdev_mbus_code_enum *code)
-{
- if (code->pad >= VDIC_NUM_PADS)
- return -EINVAL;
-
- if (code->pad == VDIC_SINK_PAD_IDMAC)
- return imx_media_enum_format(NULL, &code->code, code->index,
- false, false);
-
- return imx_media_enum_ipu_format(NULL, &code->code, code->index, false);
-}
-
static struct v4l2_mbus_framefmt *
__vdic_get_fmt(struct vdic_priv *priv, struct v4l2_subdev_pad_config *cfg,
unsigned int pad, enum v4l2_subdev_format_whence which)
@@ -541,6 +527,16 @@ __vdic_get_fmt(struct vdic_priv *priv, struct v4l2_subdev_pad_config *cfg,
return &priv->format_mbus[pad];
}
+static int vdic_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ if (code->pad >= VDIC_NUM_PADS)
+ return -EINVAL;
+
+ return imx_media_enum_ipu_format(&code->code, code->index, CS_SEL_YUV);
+}
+
static int vdic_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_pad_config *cfg,
struct v4l2_subdev_format *sdformat)
@@ -586,49 +582,28 @@ static int vdic_set_fmt(struct v4l2_subdev *sd,
goto out;
}
- v4l_bound_align_image(&sdformat->format.width, MIN_W, MAX_W_VDIC,
- W_ALIGN, &sdformat->format.height,
- MIN_H, MAX_H_VDIC, H_ALIGN, S_ALIGN);
+ cc = imx_media_find_ipu_format(sdformat->format.code, CS_SEL_YUV);
+ if (!cc) {
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
+ cc = imx_media_find_ipu_format(code, CS_SEL_YUV);
+ sdformat->format.code = cc->codes[0];
+ }
switch (sdformat->pad) {
case VDIC_SRC_PAD_DIRECT:
infmt = __vdic_get_fmt(priv, cfg, priv->active_input_pad,
sdformat->which);
-
- cc = imx_media_find_ipu_format(0, sdformat->format.code, false);
- if (!cc) {
- imx_media_enum_ipu_format(NULL, &code, 0, false);
- cc = imx_media_find_ipu_format(0, code, false);
- sdformat->format.code = cc->codes[0];
- }
-
sdformat->format.width = infmt->width;
sdformat->format.height = infmt->height;
/* output is always progressive! */
sdformat->format.field = V4L2_FIELD_NONE;
break;
- case VDIC_SINK_PAD_IDMAC:
case VDIC_SINK_PAD_DIRECT:
- if (sdformat->pad == VDIC_SINK_PAD_DIRECT) {
- cc = imx_media_find_ipu_format(0, sdformat->format.code,
- false);
- if (!cc) {
- imx_media_enum_ipu_format(NULL, &code, 0,
- false);
- cc = imx_media_find_ipu_format(0, code, false);
- sdformat->format.code = cc->codes[0];
- }
- } else {
- cc = imx_media_find_format(0, sdformat->format.code,
- false, false);
- if (!cc) {
- imx_media_enum_format(NULL, &code, 0,
- false, false);
- cc = imx_media_find_format(0, code,
- false, false);
- sdformat->format.code = cc->codes[0];
- }
- }
+ case VDIC_SINK_PAD_IDMAC:
+ v4l_bound_align_image(&sdformat->format.width,
+ MIN_W, MAX_W_VDIC, W_ALIGN,
+ &sdformat->format.height,
+ MIN_H, MAX_H_VDIC, H_ALIGN, S_ALIGN);
/* input must be interlaced! Choose SEQ_TB if not */
if (!V4L2_FIELD_HAS_BOTH(sdformat->format.field))
@@ -814,7 +789,7 @@ static int vdic_registered(struct v4l2_subdev *sd)
code = 0;
if (i != VDIC_SINK_PAD_IDMAC)
- imx_media_enum_ipu_format(NULL, &code, 0, true);
+ imx_media_enum_ipu_format(&code, 0, CS_SEL_YUV);
/* set a default mbus format */
ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
diff --git a/drivers/staging/media/imx/imx-media.h b/drivers/staging/media/imx/imx-media.h
index a67ee14..2342422 100644
--- a/drivers/staging/media/imx/imx-media.h
+++ b/drivers/staging/media/imx/imx-media.h
@@ -171,16 +171,23 @@ struct imx_media_dev {
struct v4l2_async_notifier subdev_notifier;
};
-const struct imx_media_pixfmt *imx_media_find_format(u32 fourcc, u32 code,
- bool allow_rgb,
- bool allow_planar);
-const struct imx_media_pixfmt *imx_media_find_ipu_format(u32 fourcc, u32 code,
- bool allow_rgb);
-
-int imx_media_enum_format(u32 *fourcc, u32 *code, u32 index,
- bool allow_rgb, bool allow_planar);
-int imx_media_enum_ipu_format(u32 *fourcc, u32 *code, u32 index,
- bool allow_rgb);
+enum codespace_sel {
+ CS_SEL_YUV = 0,
+ CS_SEL_RGB,
+ CS_SEL_ANY,
+};
+
+const struct imx_media_pixfmt *
+imx_media_find_format(u32 fourcc, enum codespace_sel cs_sel);
+int imx_media_enum_format(u32 *fourcc, u32 index, enum codespace_sel cs_sel);
+const struct imx_media_pixfmt *
+imx_media_find_mbus_format(u32 code, enum codespace_sel cs_sel,
+ bool allow_bayer);
+int imx_media_enum_mbus_format(u32 *code, u32 index, enum codespace_sel cs_sel,
+ bool allow_bayer);
+const struct imx_media_pixfmt *
+imx_media_find_ipu_format(u32 code, enum codespace_sel cs_sel);
+int imx_media_enum_ipu_format(u32 *code, u32 index, enum codespace_sel cs_sel);
int imx_media_init_mbus_fmt(struct v4l2_mbus_framefmt *mbus,
u32 width, u32 height, u32 code, u32 field,
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-10 06:00 +0100 |
| Subject | [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjkOL-708-59@gated-at.bofh.it> |
| In reply to | #1596626 |
Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
output device has measured an interval between the reception or transmit
completion of two consecutive frames of video that is outside the nominal
frame interval by some tolerance value.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++++++
Documentation/media/videodev2.h.rst.exceptions | 1 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 8 insertions(+)
diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
index 8d663a7..dc77363 100644
--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
@@ -197,6 +197,12 @@ call.
the regions changes. This event has a struct
:c:type:`v4l2_event_motion_det`
associated with it.
+ * - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
+ - 7
+ - This event is triggered when the video capture or output device
+ has measured an interval between the reception or transmit
+ completion of two consecutive frames of video that is outside
+ the nominal frame interval by some tolerance value.
* - ``V4L2_EVENT_PRIVATE_START``
- 0x08000000
- Base event number for driver-private events.
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index e11a0d0..c7d8fad 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
replace define V4L2_EVENT_FRAME_SYNC event-type
replace define V4L2_EVENT_SOURCE_CHANGE event-type
replace define V4L2_EVENT_MOTION_DET event-type
+replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
replace define V4L2_EVENT_PRIVATE_START event-type
replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 45184a2..cf5a0d0 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
#define V4L2_EVENT_FRAME_SYNC 4
#define V4L2_EVENT_SOURCE_CHANGE 5
#define V4L2_EVENT_MOTION_DET 6
+#define V4L2_EVENT_FRAME_INTERVAL_ERROR 7
#define V4L2_EVENT_PRIVATE_START 0x08000000
/* Payload for V4L2_EVENT_VSYNC */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-10 13:10 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjrwS-3LF-43@gated-at.bofh.it> |
| In reply to | #1596648 |
On 10/03/17 05:52, Steve Longerbeam wrote:
> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
> output device has measured an interval between the reception or transmit
> completion of two consecutive frames of video that is outside the nominal
> frame interval by some tolerance value.
Reading back what was said on this I agree with Sakari that this doesn't
belong here.
Userspace can detect this just as easily (if not easier) with a timeout.
Regards,
Hans
>
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
> Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++++++
> Documentation/media/videodev2.h.rst.exceptions | 1 +
> include/uapi/linux/videodev2.h | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> index 8d663a7..dc77363 100644
> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
> @@ -197,6 +197,12 @@ call.
> the regions changes. This event has a struct
> :c:type:`v4l2_event_motion_det`
> associated with it.
> + * - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
> + - 7
> + - This event is triggered when the video capture or output device
> + has measured an interval between the reception or transmit
> + completion of two consecutive frames of video that is outside
> + the nominal frame interval by some tolerance value.
> * - ``V4L2_EVENT_PRIVATE_START``
> - 0x08000000
> - Base event number for driver-private events.
> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
> index e11a0d0..c7d8fad 100644
> --- a/Documentation/media/videodev2.h.rst.exceptions
> +++ b/Documentation/media/videodev2.h.rst.exceptions
> @@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
> replace define V4L2_EVENT_FRAME_SYNC event-type
> replace define V4L2_EVENT_SOURCE_CHANGE event-type
> replace define V4L2_EVENT_MOTION_DET event-type
> +replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
> replace define V4L2_EVENT_PRIVATE_START event-type
>
> replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 45184a2..cf5a0d0 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
> #define V4L2_EVENT_FRAME_SYNC 4
> #define V4L2_EVENT_SOURCE_CHANGE 5
> #define V4L2_EVENT_MOTION_DET 6
> +#define V4L2_EVENT_FRAME_INTERVAL_ERROR 7
> #define V4L2_EVENT_PRIVATE_START 0x08000000
>
> /* Payload for V4L2_EVENT_VSYNC */
>
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-10 19:40 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjxCi-7Rw-23@gated-at.bofh.it> |
| In reply to | #1597268 |
Hi Hans,
On 03/10/2017 04:03 AM, Hans Verkuil wrote:
> On 10/03/17 05:52, Steve Longerbeam wrote:
>> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
>> output device has measured an interval between the reception or transmit
>> completion of two consecutive frames of video that is outside the nominal
>> frame interval by some tolerance value.
>
> Reading back what was said on this I agree with Sakari that this doesn't
> belong here.
>
> Userspace can detect this just as easily (if not easier) with a timeout.
>
Unfortunately measuring frame intervals from userland is not accurate
enough for i.MX6.
The issue here is that the IPUv3, specifically the CSI unit, can
permanently lose vertical sync if there are truncated frames sent
on the bt.656 bus. We have seen a single missing line of video cause
loss of vertical sync. The only way to correct this is to shutdown
the IPU capture hardware and restart, which can be accomplished
simply by restarting streaming from userland.
There are no other indicators from the sensor about these short
frame events (believe me, we've exhausted all avenues with the ADV718x).
And the IPUv3 DMA engine has no status indicators for short frames
either. So the only way to detect them is by measuring frame intervals.
The intervals have to be able to resolve a single line of missing video.
With a PAL video source that requires better than 58 usec accuracy.
There is too much uncertainty to resolve this at user level. The
driver is able to resolve this by measuring intervals between hardware
interrupts as long as interrupt latency is reasonably low, and we
have another method using the i.MX6 hardware input capture support
that can measure these intervals very accurately with no errors
introduced by interrupt latency.
I made this event a private event to imx-media driver in a previous
iteration, so I can return it to a private event, but this can't be
done at user level.
Steve
>
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>> Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++++++
>> Documentation/media/videodev2.h.rst.exceptions | 1 +
>> include/uapi/linux/videodev2.h | 1 +
>> 3 files changed, 8 insertions(+)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> index 8d663a7..dc77363 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>> @@ -197,6 +197,12 @@ call.
>> the regions changes. This event has a struct
>> :c:type:`v4l2_event_motion_det`
>> associated with it.
>> + * - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
>> + - 7
>> + - This event is triggered when the video capture or output device
>> + has measured an interval between the reception or transmit
>> + completion of two consecutive frames of video that is outside
>> + the nominal frame interval by some tolerance value.
>> * - ``V4L2_EVENT_PRIVATE_START``
>> - 0x08000000
>> - Base event number for driver-private events.
>> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
>> index e11a0d0..c7d8fad 100644
>> --- a/Documentation/media/videodev2.h.rst.exceptions
>> +++ b/Documentation/media/videodev2.h.rst.exceptions
>> @@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
>> replace define V4L2_EVENT_FRAME_SYNC event-type
>> replace define V4L2_EVENT_SOURCE_CHANGE event-type
>> replace define V4L2_EVENT_MOTION_DET event-type
>> +replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
>> replace define V4L2_EVENT_PRIVATE_START event-type
>>
>> replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>> index 45184a2..cf5a0d0 100644
>> --- a/include/uapi/linux/videodev2.h
>> +++ b/include/uapi/linux/videodev2.h
>> @@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
>> #define V4L2_EVENT_FRAME_SYNC 4
>> #define V4L2_EVENT_SOURCE_CHANGE 5
>> #define V4L2_EVENT_MOTION_DET 6
>> +#define V4L2_EVENT_FRAME_INTERVAL_ERROR 7
>> #define V4L2_EVENT_PRIVATE_START 0x08000000
>>
>> /* Payload for V4L2_EVENT_VSYNC */
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-03-11 00:40 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjCiB-2B7-9@gated-at.bofh.it> |
| In reply to | #1598017 |
[Multipart message — attachments visible in raw view] — view raw
On Fri 2017-03-10 10:37:21, Steve Longerbeam wrote: > Hi Hans, > > On 03/10/2017 04:03 AM, Hans Verkuil wrote: > >On 10/03/17 05:52, Steve Longerbeam wrote: > >>Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or > >>output device has measured an interval between the reception or transmit > >>completion of two consecutive frames of video that is outside the nominal > >>frame interval by some tolerance value. > > > >Reading back what was said on this I agree with Sakari that this doesn't > >belong here. > > > >Userspace can detect this just as easily (if not easier) with a timeout. > > > > > Unfortunately measuring frame intervals from userland is not accurate > enough for i.MX6. > > The issue here is that the IPUv3, specifically the CSI unit, can > permanently lose vertical sync if there are truncated frames sent > on the bt.656 bus. We have seen a single missing line of video cause > loss of vertical sync. The only way to correct this is to shutdown > the IPU capture hardware and restart, which can be accomplished > simply by restarting streaming from userland. > > There are no other indicators from the sensor about these short > frame events (believe me, we've exhausted all avenues with the ADV718x). > And the IPUv3 DMA engine has no status indicators for short frames > either. So the only way to detect them is by measuring frame intervals. > > The intervals have to be able to resolve a single line of missing video. > With a PAL video source that requires better than 58 usec accuracy. > > There is too much uncertainty to resolve this at user level. The > driver is able to resolve this by measuring intervals between hardware > interrupts as long as interrupt latency is reasonably low, and we > have another method using the i.MX6 hardware input capture support > that can measure these intervals very accurately with no errors > introduced by interrupt latency. Requiring < 58 usec interrupt latency for correct operation is a little too optimistic, no? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-11 00:50 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjCsh-2Fo-1@gated-at.bofh.it> |
| In reply to | #1598178 |
On 03/10/2017 03:30 PM, Pavel Machek wrote: > On Fri 2017-03-10 10:37:21, Steve Longerbeam wrote: >> Hi Hans, >> >> On 03/10/2017 04:03 AM, Hans Verkuil wrote: >>> On 10/03/17 05:52, Steve Longerbeam wrote: >>>> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or >>>> output device has measured an interval between the reception or transmit >>>> completion of two consecutive frames of video that is outside the nominal >>>> frame interval by some tolerance value. >>> >>> Reading back what was said on this I agree with Sakari that this doesn't >>> belong here. >>> >>> Userspace can detect this just as easily (if not easier) with a timeout. >>> >> >> >> Unfortunately measuring frame intervals from userland is not accurate >> enough for i.MX6. >> >> The issue here is that the IPUv3, specifically the CSI unit, can >> permanently lose vertical sync if there are truncated frames sent >> on the bt.656 bus. We have seen a single missing line of video cause >> loss of vertical sync. The only way to correct this is to shutdown >> the IPU capture hardware and restart, which can be accomplished >> simply by restarting streaming from userland. >> >> There are no other indicators from the sensor about these short >> frame events (believe me, we've exhausted all avenues with the ADV718x). >> And the IPUv3 DMA engine has no status indicators for short frames >> either. So the only way to detect them is by measuring frame intervals. >> >> The intervals have to be able to resolve a single line of missing video. >> With a PAL video source that requires better than 58 usec accuracy. >> >> There is too much uncertainty to resolve this at user level. The >> driver is able to resolve this by measuring intervals between hardware >> interrupts as long as interrupt latency is reasonably low, and we >> have another method using the i.MX6 hardware input capture support >> that can measure these intervals very accurately with no errors >> introduced by interrupt latency. > > Requiring < 58 usec interrupt latency for correct operation is a > little too optimistic, no? No it's not too optimistic, from experience the imx6 kernel has irq latency less than 10 usec under normal system load. False events can be generated if the latency gets bad, it's true, and that's why there is the imx6 timer input capture approach. Steve
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-11 12:50 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjNH3-2cd-7@gated-at.bofh.it> |
| In reply to | #1598017 |
On 10/03/17 19:37, Steve Longerbeam wrote:
> Hi Hans,
>
> On 03/10/2017 04:03 AM, Hans Verkuil wrote:
>> On 10/03/17 05:52, Steve Longerbeam wrote:
>>> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or
>>> output device has measured an interval between the reception or transmit
>>> completion of two consecutive frames of video that is outside the nominal
>>> frame interval by some tolerance value.
>>
>> Reading back what was said on this I agree with Sakari that this doesn't
>> belong here.
>>
>> Userspace can detect this just as easily (if not easier) with a timeout.
>>
>
>
> Unfortunately measuring frame intervals from userland is not accurate
> enough for i.MX6.
>
> The issue here is that the IPUv3, specifically the CSI unit, can
> permanently lose vertical sync if there are truncated frames sent
> on the bt.656 bus. We have seen a single missing line of video cause
> loss of vertical sync. The only way to correct this is to shutdown
> the IPU capture hardware and restart, which can be accomplished
> simply by restarting streaming from userland.
>
> There are no other indicators from the sensor about these short
> frame events (believe me, we've exhausted all avenues with the ADV718x).
> And the IPUv3 DMA engine has no status indicators for short frames
> either. So the only way to detect them is by measuring frame intervals.
>
> The intervals have to be able to resolve a single line of missing video.
> With a PAL video source that requires better than 58 usec accuracy.
>
> There is too much uncertainty to resolve this at user level. The
> driver is able to resolve this by measuring intervals between hardware
> interrupts as long as interrupt latency is reasonably low, and we
> have another method using the i.MX6 hardware input capture support
> that can measure these intervals very accurately with no errors
> introduced by interrupt latency.
>
> I made this event a private event to imx-media driver in a previous
> iteration, so I can return it to a private event, but this can't be
> done at user level.
It's fine to use an internal event as long as the end-user doesn't
see it. But if you lose vsyncs, then you never capture another frame,
right? So userspace can detect that (i.e. no new frames arrive) and
it can timeout on that. Or you detect it in the driver and restart there,
or call vb2_queue_error().
Anything really as long as this event isn't user-visible :-)
Regards,
Hans
>
> Steve
>
>
>>
>>>
>>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>>> ---
>>> Documentation/media/uapi/v4l/vidioc-dqevent.rst | 6 ++++++
>>> Documentation/media/videodev2.h.rst.exceptions | 1 +
>>> include/uapi/linux/videodev2.h | 1 +
>>> 3 files changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> index 8d663a7..dc77363 100644
>>> --- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> +++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
>>> @@ -197,6 +197,12 @@ call.
>>> the regions changes. This event has a struct
>>> :c:type:`v4l2_event_motion_det`
>>> associated with it.
>>> + * - ``V4L2_EVENT_FRAME_INTERVAL_ERROR``
>>> + - 7
>>> + - This event is triggered when the video capture or output device
>>> + has measured an interval between the reception or transmit
>>> + completion of two consecutive frames of video that is outside
>>> + the nominal frame interval by some tolerance value.
>>> * - ``V4L2_EVENT_PRIVATE_START``
>>> - 0x08000000
>>> - Base event number for driver-private events.
>>> diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
>>> index e11a0d0..c7d8fad 100644
>>> --- a/Documentation/media/videodev2.h.rst.exceptions
>>> +++ b/Documentation/media/videodev2.h.rst.exceptions
>>> @@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
>>> replace define V4L2_EVENT_FRAME_SYNC event-type
>>> replace define V4L2_EVENT_SOURCE_CHANGE event-type
>>> replace define V4L2_EVENT_MOTION_DET event-type
>>> +replace define V4L2_EVENT_FRAME_INTERVAL_ERROR event-type
>>> replace define V4L2_EVENT_PRIVATE_START event-type
>>>
>>> replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
>>> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
>>> index 45184a2..cf5a0d0 100644
>>> --- a/include/uapi/linux/videodev2.h
>>> +++ b/include/uapi/linux/videodev2.h
>>> @@ -2131,6 +2131,7 @@ struct v4l2_streamparm {
>>> #define V4L2_EVENT_FRAME_SYNC 4
>>> #define V4L2_EVENT_SOURCE_CHANGE 5
>>> #define V4L2_EVENT_MOTION_DET 6
>>> +#define V4L2_EVENT_FRAME_INTERVAL_ERROR 7
>>> #define V4L2_EVENT_PRIVATE_START 0x08000000
>>>
>>> /* Payload for V4L2_EVENT_VSYNC */
>>>
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-11 19:20 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjTMt-6AE-3@gated-at.bofh.it> |
| In reply to | #1598307 |
On 03/11/2017 03:39 AM, Hans Verkuil wrote: > On 10/03/17 19:37, Steve Longerbeam wrote: >> Hi Hans, >> >> On 03/10/2017 04:03 AM, Hans Verkuil wrote: >>> On 10/03/17 05:52, Steve Longerbeam wrote: >>>> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or >>>> output device has measured an interval between the reception or transmit >>>> completion of two consecutive frames of video that is outside the nominal >>>> frame interval by some tolerance value. >>> >>> Reading back what was said on this I agree with Sakari that this doesn't >>> belong here. >>> >>> Userspace can detect this just as easily (if not easier) with a timeout. >>> >> >> >> Unfortunately measuring frame intervals from userland is not accurate >> enough for i.MX6. >> >> The issue here is that the IPUv3, specifically the CSI unit, can >> permanently lose vertical sync if there are truncated frames sent >> on the bt.656 bus. We have seen a single missing line of video cause >> loss of vertical sync. The only way to correct this is to shutdown >> the IPU capture hardware and restart, which can be accomplished >> simply by restarting streaming from userland. >> >> There are no other indicators from the sensor about these short >> frame events (believe me, we've exhausted all avenues with the ADV718x). >> And the IPUv3 DMA engine has no status indicators for short frames >> either. So the only way to detect them is by measuring frame intervals. >> >> The intervals have to be able to resolve a single line of missing video. >> With a PAL video source that requires better than 58 usec accuracy. >> >> There is too much uncertainty to resolve this at user level. The >> driver is able to resolve this by measuring intervals between hardware >> interrupts as long as interrupt latency is reasonably low, and we >> have another method using the i.MX6 hardware input capture support >> that can measure these intervals very accurately with no errors >> introduced by interrupt latency. >> >> I made this event a private event to imx-media driver in a previous >> iteration, so I can return it to a private event, but this can't be >> done at user level. > > It's fine to use an internal event as long as the end-user doesn't > see it. But if you lose vsyncs, then you never capture another frame, > right? No, that's not correct. By loss of vertical sync, I mean the IPU captures portions of two different frames, resulting in a permanent "split image", with one frame containing portions of two consecutive images. Or, the video rolls continuously, if you remember the old CRT television sets of yore, it's the same rolling effect. > So userspace can detect that (i.e. no new frames arrive) and > it can timeout on that. Or you detect it in the driver and restart there, > or call vb2_queue_error(). > There is no timeout, the frames keep coming, but they are split images or rolling. > Anything really as long as this event isn't user-visible :-) The event must be user visible, otherwise the user has no indication the error, and can't correct it by stream restart. Steve
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2017-03-11 20:00 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjUpc-6PV-5@gated-at.bofh.it> |
| In reply to | #1598396 |
On Sat, Mar 11, 2017 at 10:14:49AM -0800, Steve Longerbeam wrote: > On 03/11/2017 03:39 AM, Hans Verkuil wrote: > >It's fine to use an internal event as long as the end-user doesn't > >see it. But if you lose vsyncs, then you never capture another frame, > >right? > > No, that's not correct. By loss of vertical sync, I mean the IPU > captures portions of two different frames, resulting in a permanent > "split image", with one frame containing portions of two consecutive > images. Or, the video rolls continuously, if you remember the old CRT > television sets of yore, it's the same rolling effect. I have seen that rolling effect, but the iMX6 regains correct sync within one complete "roll" just fine here with IMX219. However, it has always recovered. So, I don't think there's a problem with the iMX6 part of the processing, and so I don't think we should cripple the iMX6 capture drivers for this problem. It seems to me that the problem is with the source. -- 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.
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-11 20:00 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjUpc-6PV-9@gated-at.bofh.it> |
| In reply to | #1598418 |
On 03/11/2017 10:51 AM, Russell King - ARM Linux wrote: > On Sat, Mar 11, 2017 at 10:14:49AM -0800, Steve Longerbeam wrote: >> On 03/11/2017 03:39 AM, Hans Verkuil wrote: >>> It's fine to use an internal event as long as the end-user doesn't >>> see it. But if you lose vsyncs, then you never capture another frame, >>> right? >> >> No, that's not correct. By loss of vertical sync, I mean the IPU >> captures portions of two different frames, resulting in a permanent >> "split image", with one frame containing portions of two consecutive >> images. Or, the video rolls continuously, if you remember the old CRT >> television sets of yore, it's the same rolling effect. > > I have seen that rolling effect, but the iMX6 regains correct sync > within one complete "roll" just fine here with IMX219. However, it > has always recovered. I've seen permanent split images, and rolling that continues for minutes. > > So, I don't think there's a problem with the iMX6 part of the > processing, and so I don't think we should cripple the iMX6 capture > drivers for this problem. > > It seems to me that the problem is with the source. The the problem is almost surely in the CSI. It's handling of the bt.656 sync codes is broken in some way. Steve
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-11 20:10 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tjUyS-78s-19@gated-at.bofh.it> |
| In reply to | #1598418 |
On 03/11/2017 10:51 AM, Russell King - ARM Linux wrote: > On Sat, Mar 11, 2017 at 10:14:49AM -0800, Steve Longerbeam wrote: >> On 03/11/2017 03:39 AM, Hans Verkuil wrote: >>> It's fine to use an internal event as long as the end-user doesn't >>> see it. But if you lose vsyncs, then you never capture another frame, >>> right? >> >> No, that's not correct. By loss of vertical sync, I mean the IPU >> captures portions of two different frames, resulting in a permanent >> "split image", with one frame containing portions of two consecutive >> images. Or, the video rolls continuously, if you remember the old CRT >> television sets of yore, it's the same rolling effect. > > I have seen that rolling effect, but the iMX6 regains correct sync > within one complete "roll" just fine here with IMX219. However, it > has always recovered. > > So, I don't think there's a problem with the iMX6 part of the > processing, and so I don't think we should cripple the iMX6 capture > drivers for this problem. And there is no crippling going on. Measuring the frame intervals adds very little overhead. Steve
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-13 11:10 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tkv5o-7pV-25@gated-at.bofh.it> |
| In reply to | #1598396 |
On 03/11/2017 07:14 PM, Steve Longerbeam wrote: > > > On 03/11/2017 03:39 AM, Hans Verkuil wrote: >> On 10/03/17 19:37, Steve Longerbeam wrote: >>> Hi Hans, >>> >>> On 03/10/2017 04:03 AM, Hans Verkuil wrote: >>>> On 10/03/17 05:52, Steve Longerbeam wrote: >>>>> Add a new FRAME_INTERVAL_ERROR event to signal that a video capture or >>>>> output device has measured an interval between the reception or transmit >>>>> completion of two consecutive frames of video that is outside the nominal >>>>> frame interval by some tolerance value. >>>> >>>> Reading back what was said on this I agree with Sakari that this doesn't >>>> belong here. >>>> >>>> Userspace can detect this just as easily (if not easier) with a timeout. >>>> >>> >>> >>> Unfortunately measuring frame intervals from userland is not accurate >>> enough for i.MX6. >>> >>> The issue here is that the IPUv3, specifically the CSI unit, can >>> permanently lose vertical sync if there are truncated frames sent >>> on the bt.656 bus. We have seen a single missing line of video cause >>> loss of vertical sync. The only way to correct this is to shutdown >>> the IPU capture hardware and restart, which can be accomplished >>> simply by restarting streaming from userland. >>> >>> There are no other indicators from the sensor about these short >>> frame events (believe me, we've exhausted all avenues with the ADV718x). >>> And the IPUv3 DMA engine has no status indicators for short frames >>> either. So the only way to detect them is by measuring frame intervals. >>> >>> The intervals have to be able to resolve a single line of missing video. >>> With a PAL video source that requires better than 58 usec accuracy. >>> >>> There is too much uncertainty to resolve this at user level. The >>> driver is able to resolve this by measuring intervals between hardware >>> interrupts as long as interrupt latency is reasonably low, and we >>> have another method using the i.MX6 hardware input capture support >>> that can measure these intervals very accurately with no errors >>> introduced by interrupt latency. >>> >>> I made this event a private event to imx-media driver in a previous >>> iteration, so I can return it to a private event, but this can't be >>> done at user level. >> >> It's fine to use an internal event as long as the end-user doesn't >> see it. But if you lose vsyncs, then you never capture another frame, >> right? > > No, that's not correct. By loss of vertical sync, I mean the IPU > captures portions of two different frames, resulting in a permanent > "split image", with one frame containing portions of two consecutive > images. Or, the video rolls continuously, if you remember the old CRT > television sets of yore, it's the same rolling effect. > > >> So userspace can detect that (i.e. no new frames arrive) and >> it can timeout on that. Or you detect it in the driver and restart there, >> or call vb2_queue_error(). >> > > There is no timeout, the frames keep coming, but they are split images > or rolling. Ah, OK. That wasn't clear to me from the description. > >> Anything really as long as this event isn't user-visible :-) > > The event must be user visible, otherwise the user has no indication > the error, and can't correct it by stream restart. In that case the driver can detect this and call vb2_queue_error. It's what it is there for. The event doesn't help you since only this driver has this issue. So nobody will watch this event, unless it is sw specifically written for this SoC. Much better to call vb2_queue_error to signal a fatal error (which this apparently is) since there are more drivers that do this, and vivid supports triggering this condition as well. Regards, Hans
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2017-03-13 11:50 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tkvI6-7IV-13@gated-at.bofh.it> |
| In reply to | #1599169 |
On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote: > On 03/11/2017 07:14 PM, Steve Longerbeam wrote: > > The event must be user visible, otherwise the user has no indication > > the error, and can't correct it by stream restart. > > In that case the driver can detect this and call vb2_queue_error. It's > what it is there for. > > The event doesn't help you since only this driver has this issue. So nobody > will watch this event, unless it is sw specifically written for this SoC. > > Much better to call vb2_queue_error to signal a fatal error (which this > apparently is) since there are more drivers that do this, and vivid supports > triggering this condition as well. So today, I can fiddle around with the IMX219 registers to help gain an understanding of how this sensor works. Several of the registers (such as the PLL setup [*]) require me to disable streaming on the sensor while changing them. This is something I've done many times while testing various ideas, and is my primary way of figuring out and testing such things. Whenever I resume streaming (provided I've let the sensor stop streaming at a frame boundary) it resumes as if nothing happened. If I stop the sensor mid-frame, then I get the rolling issue that Steve reports, but once the top of the frame becomes aligned with the top of the capture, everything then becomes stable again as if nothing happened. The side effect of what you're proposing is that when I disable streaming at the sensor by poking at its registers, rather than the capture just stopping, an error is going to be delivered to gstreamer, and gstreamer is going to exit, taking the entire capture process down. This severely restricts the ability to be able to develop and test sensor drivers. So, I strongly disagree with you. Loss of capture frames is not necessarily a fatal error - as I have been saying repeatedly. In Steve's case, there's some unknown interaction between the source and iMX6 hardware that is causing the instability, but that is simply not true of other sources, and I oppose any idea that we should cripple the iMX6 side of the capture based upon just one hardware combination where this is a problem. Steve suggested that the problem could be in the iMX6 CSI block - and I note comparing Steve's code with the code in FSL's repository that there are some changes that are missing in Steve's code to do with the CCIR656 sync code setup, particularly for >8 bit. The progressive CCIR656 8-bit setup looks pretty similar though - but I think what needs to be asked is whether the same problem is visible using the FSL/NXP vendor kernel. * - the PLL setup is something that requires research at the moment. Sony's official position (even to their customers) is that they do not supply the necessary information, instead they expect customers to tell them the capture settings they want, and Sony will throw the values into a spreadsheet, and they'll supply the register settings back to the customer. Hence, the only way to proceed with a generic driver for this sensor is to experiment, and experimenting requires the ability to pause the stream at the sensor while making changes. Take this away, and we're stuck with the tables-of-register-settings-for-set-of-fixed- capture-settings approach. I've made a lot of progress away from this which is all down to the flexibility afforded by _not_ killing the capture process. -- 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.
[toc] | [prev] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2017-03-13 12:00 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tkvRM-7Mz-21@gated-at.bofh.it> |
| In reply to | #1599191 |
On 03/13/2017 11:45 AM, Russell King - ARM Linux wrote: > On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote: >> On 03/11/2017 07:14 PM, Steve Longerbeam wrote: >>> The event must be user visible, otherwise the user has no indication >>> the error, and can't correct it by stream restart. >> >> In that case the driver can detect this and call vb2_queue_error. It's >> what it is there for. >> >> The event doesn't help you since only this driver has this issue. So nobody >> will watch this event, unless it is sw specifically written for this SoC. >> >> Much better to call vb2_queue_error to signal a fatal error (which this >> apparently is) since there are more drivers that do this, and vivid supports >> triggering this condition as well. > > So today, I can fiddle around with the IMX219 registers to help gain > an understanding of how this sensor works. Several of the registers > (such as the PLL setup [*]) require me to disable streaming on the > sensor while changing them. > > This is something I've done many times while testing various ideas, > and is my primary way of figuring out and testing such things. > > Whenever I resume streaming (provided I've let the sensor stop > streaming at a frame boundary) it resumes as if nothing happened. If I > stop the sensor mid-frame, then I get the rolling issue that Steve > reports, but once the top of the frame becomes aligned with the top of > the capture, everything then becomes stable again as if nothing happened. > > The side effect of what you're proposing is that when I disable streaming > at the sensor by poking at its registers, rather than the capture just > stopping, an error is going to be delivered to gstreamer, and gstreamer > is going to exit, taking the entire capture process down. > > This severely restricts the ability to be able to develop and test > sensor drivers. > > So, I strongly disagree with you. > > Loss of capture frames is not necessarily a fatal error - as I have been > saying repeatedly. In Steve's case, there's some unknown interaction > between the source and iMX6 hardware that is causing the instability, > but that is simply not true of other sources, and I oppose any idea that > we should cripple the iMX6 side of the capture based upon just one > hardware combination where this is a problem. > > Steve suggested that the problem could be in the iMX6 CSI block - and I > note comparing Steve's code with the code in FSL's repository that there > are some changes that are missing in Steve's code to do with the CCIR656 > sync code setup, particularly for >8 bit. The progressive CCIR656 8-bit > setup looks pretty similar though - but I think what needs to be asked > is whether the same problem is visible using the FSL/NXP vendor kernel. > > > * - the PLL setup is something that requires research at the moment. > Sony's official position (even to their customers) is that they do not > supply the necessary information, instead they expect customers to tell > them the capture settings they want, and Sony will throw the values into > a spreadsheet, and they'll supply the register settings back to the > customer. Hence, the only way to proceed with a generic driver for > this sensor is to experiment, and experimenting requires the ability to > pause the stream at the sensor while making changes. Take this away, > and we're stuck with the tables-of-register-settings-for-set-of-fixed- > capture-settings approach. I've made a lot of progress away from this > which is all down to the flexibility afforded by _not_ killing the > capture process. > In other words: Steve should either find a proper fix for this, or only call vb2_queue_error in this specific case. Sending an event that nobody will know how to handle or what to do with is pretty pointless IMHO. Let's just give him time to try and figure out the real issue here. Regards, Hans
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-03-13 18:10 +0100 |
| Subject | Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event |
| Message-ID | <tkBDQ-3Ed-9@gated-at.bofh.it> |
| In reply to | #1599207 |
On 03/13/2017 03:53 AM, Hans Verkuil wrote: > On 03/13/2017 11:45 AM, Russell King - ARM Linux wrote: >> On Mon, Mar 13, 2017 at 11:02:34AM +0100, Hans Verkuil wrote: >>> On 03/11/2017 07:14 PM, Steve Longerbeam wrote: >>>> The event must be user visible, otherwise the user has no indication >>>> the error, and can't correct it by stream restart. >>> >>> In that case the driver can detect this and call vb2_queue_error. It's >>> what it is there for. >>> >>> The event doesn't help you since only this driver has this issue. So nobody >>> will watch this event, unless it is sw specifically written for this SoC. >>> >>> Much better to call vb2_queue_error to signal a fatal error (which this >>> apparently is) since there are more drivers that do this, and vivid supports >>> triggering this condition as well. >> >> So today, I can fiddle around with the IMX219 registers to help gain >> an understanding of how this sensor works. Several of the registers >> (such as the PLL setup [*]) require me to disable streaming on the >> sensor while changing them. >> >> This is something I've done many times while testing various ideas, >> and is my primary way of figuring out and testing such things. >> >> Whenever I resume streaming (provided I've let the sensor stop >> streaming at a frame boundary) it resumes as if nothing happened. If I >> stop the sensor mid-frame, then I get the rolling issue that Steve >> reports, but once the top of the frame becomes aligned with the top of >> the capture, everything then becomes stable again as if nothing happened. >> >> The side effect of what you're proposing is that when I disable streaming >> at the sensor by poking at its registers, rather than the capture just >> stopping, an error is going to be delivered to gstreamer, and gstreamer >> is going to exit, taking the entire capture process down. >> >> This severely restricts the ability to be able to develop and test >> sensor drivers. >> >> So, I strongly disagree with you. >> >> Loss of capture frames is not necessarily a fatal error - as I have been >> saying repeatedly. In Steve's case, there's some unknown interaction >> between the source and iMX6 hardware that is causing the instability, >> but that is simply not true of other sources, and I oppose any idea that >> we should cripple the iMX6 side of the capture based upon just one >> hardware combination where this is a problem. >> >> Steve suggested that the problem could be in the iMX6 CSI block - and I >> note comparing Steve's code with the code in FSL's repository that there >> are some changes that are missing in Steve's code to do with the CCIR656 >> sync code setup, particularly for >8 bit. The progressive CCIR656 8-bit >> setup looks pretty similar though - but I think what needs to be asked >> is whether the same problem is visible using the FSL/NXP vendor kernel. >> >> >> * - the PLL setup is something that requires research at the moment. >> Sony's official position (even to their customers) is that they do not >> supply the necessary information, instead they expect customers to tell >> them the capture settings they want, and Sony will throw the values into >> a spreadsheet, and they'll supply the register settings back to the >> customer. Hence, the only way to proceed with a generic driver for >> this sensor is to experiment, and experimenting requires the ability to >> pause the stream at the sensor while making changes. Take this away, >> and we're stuck with the tables-of-register-settings-for-set-of-fixed- >> capture-settings approach. I've made a lot of progress away from this >> which is all down to the flexibility afforded by _not_ killing the >> capture process. >> > > In other words: Steve should either find a proper fix for this, or only > call vb2_queue_error in this specific case. Sending an event that nobody > will know how to handle or what to do with is pretty pointless IMHO. > > Let's just give him time to try and figure out the real issue here. This is a long-standing issue, I've traveled to Hildesheim working with our customer to try and get to the bottom of it. I can go into a lot of details from those trips, we probed the bt.656 bus with a logic analyzer and I can share those results with anyone who asks. But the results of those investigations indicate the CSI is not handling the SAV/EAV sync codes correctly - if there is a shift in the line position at which those codes occur, the CSI/IPU does not abort the frame capture DMA and start from the new sync code position, it just continues to capture lines until the programmed number of lines are transferred, hence you get these split images. Freescale also informed us of a mechanism in the IPU that will add lines if it detects these short frames, until the programmed number of lines are reached. Apparently that is what creates the rolling effect, but this rolling can last for up to a full minute, which is completely unacceptable, it must be corrected as soon as possible. So the only thing we could come up with was to monitor frame intervals, this is purely empirical, but we observed that frame intervals drop by approx. one line time (~60 usec) when these short frames are received. I don't really have any explanation for that but we take advantage of that observation by sending this event to userspace so the problem can be corrected immediately with a stream restart. As I've said, the ADV718x does not provide _any_ indication via status when the shift in the sync code position occurs. And the IPU is also severely lacking in DMA completion status as well (no short packet status like in USB for example). So the only way to detect this event is by monitoring the frame intervals. I will review differences in the CCIR code register setup from FSL's repo that Russell pointed out, but I'm fairly sure those code registers are setup correctly, there's not much room for variability in those values. They only define the values of the sync codes, so the CSI can detect them, those values are defined by the bt.656 spec. Anyway, perhaps for now I can remove the event, but keep the FI monitoring, and for now just report a kernel error message on a detected bad FI. Steve
[toc] | [prev] | [next] | [standalone]
Page 2 of 5 — ← Prev page 1 [2] 3 4 5 Next page →
Back to top | Article view | linux.kernel
csiph-web