Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1553560 > unrolled thread

[PATCH v3 00/24] i.MX Media Driver

Started bySteve Longerbeam <slongerbeam@gmail.com>
First post2017-01-07 03:20 +0100
Last post2017-01-16 14:50 +0100
Articles 11 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-07 03:20 +0100
    [PATCH v3 15/24] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-07 03:20 +0100
      Re: [PATCH v3 15/24] media: Add userspace header file for i.MX Philipp Zabel <p.zabel@pengutronix.de> - 2017-01-13 13:10 +0100
        Re: [PATCH v3 15/24] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-14 00:20 +0100
    [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-07 03:20 +0100
    [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-07 03:20 +0100
    Re: [PATCH v3 00/24] i.MX Media Driver Tim Harvey <tharvey@gateworks.com> - 2017-01-12 00:20 +0100
      Re: [PATCH v3 00/24] i.MX Media Driver Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-01-12 04:30 +0100
    Re: [PATCH v3 16/24] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-01-13 16:30 +0100
      Re: [PATCH v3 16/24] media: Add i.MX media core driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-01-14 23:50 +0100
        Re: [PATCH v3 16/24] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-01-16 14:50 +0100

#1553560 — [PATCH v3 00/24] i.MX Media Driver

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-07 03:20 +0100
Subject[PATCH v3 00/24] i.MX Media Driver
Message-ID<sWOLT-2qI-3@gated-at.bofh.it>
In version 3:

Changes suggested by Rob Herring <robh@kernel.org>:

  - prepended FIM node properties with vendor prefix "fsl,".

  - make mipi csi-2 receiver compatible string SoC specific:
    "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".

  - redundant "_clk" removed from mipi csi-2 receiver clock-names property.

  - removed board-specific info from the media driver binding doc. These
    were all related to sensor bindings, which already are (adv7180)
    or will be (ov564x) covered in separate binding docs. All reference
    board info not related to DT bindings has been moved to
    Documentation/media/v4l-drivers/imx.rst.

  - removed "_mipi" from the OV5640 compatible string.

Changes suggested by Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>:

  Mostly cosmetic/non-functional changes which I won't list here, except
  for the following:

  - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.

  - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.

  - check/handle return code from required reg property of CSI port nodes.

  - check/handle return code from clk_prepare_enable().

Changes suggested by Fabio Estevam <festevam@gmail.com>:

  - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.

  - finally got around to passing valid IOMUX pin config values to the
    pin groups.

Other changes:

  - removed the FIM properties that overrided the v4l2 FIM control defaults
    values. This was left-over from a requirement of a customer and is not
    necessary here.

  - The FIM must be explicitly enabled in the fim child node under the CSI
    port nodes, using the status property. If not enabled, FIM v4l2 controls
    will not appear in the video capture driver.

  - brought in additional media types patch from Philipp Zabel. Use new
    MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.

  - brought in latest platform generic video multiplexer subdevice driver
    from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).

  - removed imx-media-of.h, moved those prototypes into imx-media.h.


Philipp Zabel (3):
  ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their
    connections
  add mux and video interface bridge entity functions
  platform: add video-multiplexer subdevice driver

Steve Longerbeam (21):
  [media] dt-bindings: Add bindings for i.MX media driver
  ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
  ARM: dts: imx6qdl: add media device
  ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround
  ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors
  ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors
  ARM: dts: imx6-sabreauto: create i2cmux for i2c3
  ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b
  ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture
  ARM: dts: imx6-sabreauto: add the ADV7180 video decoder
  UAPI: Add media UAPI Kbuild file
  media: Add userspace header file for i.MX
  media: Add i.MX media core driver
  media: imx: Add CSI subdev driver
  media: imx: Add SMFC subdev driver
  media: imx: Add IC subdev drivers
  media: imx: Add Camera Interface subdev driver
  media: imx: Add MIPI CSI-2 Receiver subdev driver
  media: imx: Add MIPI CSI-2 OV5640 sensor subdev driver
  media: imx: Add Parallel OV5642 sensor subdev driver
  ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers

 Documentation/devicetree/bindings/media/imx.txt    |   57 +
 .../bindings/media/video-multiplexer.txt           |   59 +
 Documentation/media/uapi/mediactl/media-types.rst  |   22 +
 Documentation/media/v4l-drivers/imx.rst            |  443 ++
 arch/arm/boot/dts/imx6dl-sabrelite.dts             |    5 +
 arch/arm/boot/dts/imx6dl-sabresd.dts               |    5 +
 arch/arm/boot/dts/imx6dl.dtsi                      |  187 +
 arch/arm/boot/dts/imx6q-sabrelite.dts              |    6 +
 arch/arm/boot/dts/imx6q-sabresd.dts                |    5 +
 arch/arm/boot/dts/imx6q.dtsi                       |  127 +
 arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  147 +-
 arch/arm/boot/dts/imx6qdl-sabrelite.dtsi           |  122 +-
 arch/arm/boot/dts/imx6qdl-sabresd.dtsi             |  114 +-
 arch/arm/boot/dts/imx6qdl.dtsi                     |   25 +-
 arch/arm/configs/imx_v6_v7_defconfig               |   12 +-
 drivers/media/platform/Kconfig                     |    8 +
 drivers/media/platform/Makefile                    |    2 +
 drivers/media/platform/video-multiplexer.c         |  472 +++
 drivers/staging/media/Kconfig                      |    2 +
 drivers/staging/media/Makefile                     |    1 +
 drivers/staging/media/imx/Kconfig                  |   36 +
 drivers/staging/media/imx/Makefile                 |   15 +
 drivers/staging/media/imx/TODO                     |   22 +
 drivers/staging/media/imx/imx-camif.c              | 1000 +++++
 drivers/staging/media/imx/imx-csi.c                |  644 +++
 drivers/staging/media/imx/imx-ic-common.c          |  109 +
 drivers/staging/media/imx/imx-ic-pp.c              |  636 +++
 drivers/staging/media/imx/imx-ic-prpenc.c          | 1033 +++++
 drivers/staging/media/imx/imx-ic-prpvf.c           | 1179 ++++++
 drivers/staging/media/imx/imx-ic.h                 |   38 +
 drivers/staging/media/imx/imx-media-common.c       |  981 +++++
 drivers/staging/media/imx/imx-media-dev.c          |  486 +++
 drivers/staging/media/imx/imx-media-fim.c          |  471 +++
 drivers/staging/media/imx/imx-media-internal-sd.c  |  457 ++
 drivers/staging/media/imx/imx-media-of.c           |  289 ++
 drivers/staging/media/imx/imx-media.h              |  310 ++
 drivers/staging/media/imx/imx-mipi-csi2.c          |  501 +++
 drivers/staging/media/imx/imx-smfc.c               |  737 ++++
 drivers/staging/media/imx/ov5640-mipi.c            | 2348 +++++++++++
 drivers/staging/media/imx/ov5642.c                 | 4363 ++++++++++++++++++++
 include/media/imx.h                                |   15 +
 include/uapi/Kbuild                                |    1 +
 include/uapi/linux/media.h                         |    6 +
 include/uapi/linux/v4l2-controls.h                 |    4 +
 include/uapi/media/Kbuild                          |    2 +
 include/uapi/media/imx.h                           |   30 +
 46 files changed, 17505 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/imx.txt
 create mode 100644 Documentation/devicetree/bindings/media/video-multiplexer.txt
 create mode 100644 Documentation/media/v4l-drivers/imx.rst
 create mode 100644 drivers/media/platform/video-multiplexer.c
 create mode 100644 drivers/staging/media/imx/Kconfig
 create mode 100644 drivers/staging/media/imx/Makefile
 create mode 100644 drivers/staging/media/imx/TODO
 create mode 100644 drivers/staging/media/imx/imx-camif.c
 create mode 100644 drivers/staging/media/imx/imx-csi.c
 create mode 100644 drivers/staging/media/imx/imx-ic-common.c
 create mode 100644 drivers/staging/media/imx/imx-ic-pp.c
 create mode 100644 drivers/staging/media/imx/imx-ic-prpenc.c
 create mode 100644 drivers/staging/media/imx/imx-ic-prpvf.c
 create mode 100644 drivers/staging/media/imx/imx-ic.h
 create mode 100644 drivers/staging/media/imx/imx-media-common.c
 create mode 100644 drivers/staging/media/imx/imx-media-dev.c
 create mode 100644 drivers/staging/media/imx/imx-media-fim.c
 create mode 100644 drivers/staging/media/imx/imx-media-internal-sd.c
 create mode 100644 drivers/staging/media/imx/imx-media-of.c
 create mode 100644 drivers/staging/media/imx/imx-media.h
 create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c
 create mode 100644 drivers/staging/media/imx/imx-smfc.c
 create mode 100644 drivers/staging/media/imx/ov5640-mipi.c
 create mode 100644 drivers/staging/media/imx/ov5642.c
 create mode 100644 include/media/imx.h
 create mode 100644 include/uapi/media/Kbuild
 create mode 100644 include/uapi/media/imx.h

-- 
2.7.4

[toc] | [next] | [standalone]


#1553561 — [PATCH v3 15/24] media: Add userspace header file for i.MX

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-07 03:20 +0100
Subject[PATCH v3 15/24] media: Add userspace header file for i.MX
Message-ID<sWOLU-2qI-47@gated-at.bofh.it>
In reply to#1553560
This adds a header file for use by userspace programs wanting to interact
with the i.MX media driver. It defines custom v4l2 controls and events
generated by the i.MX v4l2 subdevices.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 include/uapi/media/Kbuild |  1 +
 include/uapi/media/imx.h  | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 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..2421d9c
--- /dev/null
+++ b/include/uapi/media/imx.h
@@ -0,0 +1,30 @@
+/*
+ * 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__
+
+/*
+ * events from the subdevs
+ */
+#define V4L2_EVENT_IMX_CLASS          V4L2_EVENT_PRIVATE_START
+#define V4L2_EVENT_IMX_NFB4EOF        (V4L2_EVENT_IMX_CLASS + 1)
+#define V4L2_EVENT_IMX_EOF_TIMEOUT    (V4L2_EVENT_IMX_CLASS + 2)
+#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
+
+enum imx_ctrl_id {
+	V4L2_CID_IMX_MOTION = (V4L2_CID_USER_IMX_BASE + 0),
+	V4L2_CID_IMX_FIM_ENABLE,
+	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]


#1558321 — Re: [PATCH v3 15/24] media: Add userspace header file for i.MX

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2017-01-13 13:10 +0100
SubjectRe: [PATCH v3 15/24] media: Add userspace header file for i.MX
Message-ID<sZ8Qc-HA-73@gated-at.bofh.it>
In reply to#1553561
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> This adds a header file for use by userspace programs wanting to interact
> with the i.MX media driver. It defines custom v4l2 controls and events
> generated by the i.MX v4l2 subdevices.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
>  include/uapi/media/Kbuild |  1 +
>  include/uapi/media/imx.h  | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 31 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..2421d9c
> --- /dev/null
> +++ b/include/uapi/media/imx.h
> @@ -0,0 +1,30 @@
> +/*
> + * 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__
> +
> +/*
> + * events from the subdevs
> + */
> +#define V4L2_EVENT_IMX_CLASS          V4L2_EVENT_PRIVATE_START
> +#define V4L2_EVENT_IMX_NFB4EOF        (V4L2_EVENT_IMX_CLASS + 1)
> +#define V4L2_EVENT_IMX_EOF_TIMEOUT    (V4L2_EVENT_IMX_CLASS + 2)
> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)

Aren't these generic enough to warrant common events? I would think
there have to be other capture IP cores that can signal aborted frames
or frame timeouts.

> +
> +enum imx_ctrl_id {
> +	V4L2_CID_IMX_MOTION = (V4L2_CID_USER_IMX_BASE + 0),
> +	V4L2_CID_IMX_FIM_ENABLE,
> +	V4L2_CID_IMX_FIM_NUM,
> +	V4L2_CID_IMX_FIM_TOLERANCE_MIN,
> +	V4L2_CID_IMX_FIM_TOLERANCE_MAX,
> +	V4L2_CID_IMX_FIM_NUM_SKIP,
> +};
> +
> +#endif

regards
Philipp

[toc] | [prev] | [next] | [standalone]


#1558795 — Re: [PATCH v3 15/24] media: Add userspace header file for i.MX

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-14 00:20 +0100
SubjectRe: [PATCH v3 15/24] media: Add userspace header file for i.MX
Message-ID<sZjix-6X3-7@gated-at.bofh.it>
In reply to#1558321

On 01/13/2017 04:05 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> This adds a header file for use by userspace programs wanting to interact
>> with the i.MX media driver. It defines custom v4l2 controls and events
>> generated by the i.MX v4l2 subdevices.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> ---
>>   include/uapi/media/Kbuild |  1 +
>>   include/uapi/media/imx.h  | 30 ++++++++++++++++++++++++++++++
>>   2 files changed, 31 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..2421d9c
>> --- /dev/null
>> +++ b/include/uapi/media/imx.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * 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__
>> +
>> +/*
>> + * events from the subdevs
>> + */
>> +#define V4L2_EVENT_IMX_CLASS          V4L2_EVENT_PRIVATE_START
>> +#define V4L2_EVENT_IMX_NFB4EOF        (V4L2_EVENT_IMX_CLASS + 1)
>> +#define V4L2_EVENT_IMX_EOF_TIMEOUT    (V4L2_EVENT_IMX_CLASS + 2)
>> +#define V4L2_EVENT_IMX_FRAME_INTERVAL (V4L2_EVENT_IMX_CLASS + 3)
> Aren't these generic enough to warrant common events? I would think
> there have to be other capture IP cores that can signal aborted frames
> or frame timeouts.

Yes, agreed. A frame capture timeout, or frame interval error, are
both generic concepts. At some point it would be great to make the
Frame Interval Monitor generally available under v4l2-core. As for the
EOF timeout event, I'll look into moving that into a generic V4L2 event.

Steve

[toc] | [prev] | [next] | [standalone]


#1553562 — [PATCH v3 20/24] media: imx: Add Camera Interface subdev driver

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-07 03:20 +0100
Subject[PATCH v3 20/24] media: imx: Add Camera Interface subdev driver
Message-ID<sWOLU-2qI-51@gated-at.bofh.it>
In reply to#1553560
This is the camera interface driver that provides the v4l2
user interface. Frames can be received from various sources:

- directly from SMFC for capturing unconverted images directly from
  camera sensors.

- from the IC pre-process encode task.

- from the IC pre-process viewfinder task.

- from the IC post-process task.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 drivers/staging/media/imx/Makefile    |    2 +-
 drivers/staging/media/imx/imx-camif.c | 1000 +++++++++++++++++++++++++++++++++
 2 files changed, 1001 insertions(+), 1 deletion(-)
 create mode 100644 drivers/staging/media/imx/imx-camif.c

diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index d2a962c..fe9e992 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -8,4 +8,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
 
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
-
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
diff --git a/drivers/staging/media/imx/imx-camif.c b/drivers/staging/media/imx/imx-camif.c
new file mode 100644
index 0000000..404f724
--- /dev/null
+++ b/drivers/staging/media/imx/imx-camif.c
@@ -0,0 +1,1000 @@
+/*
+ * Video Camera Capture Subdev for Freescale i.MX5/6 SOC
+ *
+ * Copyright (c) 2012-2016 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.
+ */
+#include <linux/delay.h>
+#include <linux/fs.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/v4l2-subdev.h>
+#include <media/videobuf2-dma-contig.h>
+#include <video/imx-ipu-v3.h>
+#include <media/imx.h>
+#include "imx-media.h"
+
+#define CAMIF_NUM_PADS 2
+
+struct camif_priv {
+	struct device         *dev;
+	struct video_device    vfd;
+	struct media_pipeline  mp;
+	struct imx_media_dev  *md;
+	struct v4l2_subdev     sd;
+	struct media_pad       pad[CAMIF_NUM_PADS];
+	struct media_pad       vd_pad;
+	int id;
+	int input_pad;
+	int output_pad;
+
+	struct v4l2_mbus_framefmt format_mbus[CAMIF_NUM_PADS];
+	const struct imx_media_pixfmt *cc[CAMIF_NUM_PADS];
+
+	/* dma buffer ring */
+	struct imx_media_dma_buf_ring *in_ring;
+	struct v4l2_subdev     *src_sd;
+
+	struct mutex           mutex;       /* capture device mutex */
+	spinlock_t             q_lock;      /* protect ready_q */
+
+	/* buffer queue used in videobuf2 */
+	struct vb2_queue       buffer_queue;
+
+	/* streaming buffer queue */
+	struct list_head       ready_q;
+
+	/* controls inherited from subdevs */
+	struct v4l2_ctrl_handler ctrl_hdlr;
+
+	/* misc status */
+	int                    current_input; /* the current input */
+	v4l2_std_id            current_std;   /* current standard */
+	bool                   stop;          /* streaming is stopping */
+};
+
+/* In bytes, per queue */
+#define VID_MEM_LIMIT	SZ_64M
+
+static struct vb2_ops camif_qops;
+
+/*
+ * Video ioctls follow
+ */
+
+static int vidioc_querycap(struct file *file, void *fh,
+			   struct v4l2_capability *cap)
+{
+	strncpy(cap->driver, "imx-media-camif", sizeof(cap->driver) - 1);
+	strncpy(cap->card, "imx-media-camif", sizeof(cap->card) - 1);
+	cap->bus_info[0] = 0;
+	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
+
+	return 0;
+}
+
+static int camif_enum_fmt_vid_cap(struct file *file, void *fh,
+				  struct v4l2_fmtdesc *f)
+{
+	const struct imx_media_pixfmt *cc;
+	u32 code;
+	int ret;
+
+	ret = imx_media_enum_format(&code, f->index, true, true);
+	if (ret)
+		return ret;
+	cc = imx_media_find_format(0, code, true, true);
+	if (!cc)
+		return -EINVAL;
+
+	f->pixelformat = cc->fourcc;
+
+	return 0;
+}
+
+static int camif_g_fmt_vid_cap(struct file *file, void *fh,
+			       struct v4l2_format *f)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct v4l2_mbus_framefmt *outfmt;
+
+	/* user format is the same as the format from output pad */
+	outfmt = &priv->format_mbus[priv->output_pad];
+	return imx_media_mbus_fmt_to_pix_fmt(&f->fmt.pix, outfmt);
+}
+
+static int camif_try_fmt_vid_cap(struct file *file, void *fh,
+				 struct v4l2_format *f)
+{
+	return camif_g_fmt_vid_cap(file, fh, f);
+}
+
+static int camif_s_fmt_vid_cap(struct file *file, void *fh,
+			       struct v4l2_format *f)
+{
+	struct camif_priv *priv = video_drvdata(file);
+
+	if (vb2_is_busy(&priv->buffer_queue)) {
+		v4l2_err(&priv->sd, "%s queue busy\n", __func__);
+		return -EBUSY;
+	}
+
+	return camif_try_fmt_vid_cap(file, priv, f);
+}
+
+static int camif_querystd(struct file *file, void *fh, v4l2_std_id *std)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	return v4l2_subdev_call(sensor->sd, video, querystd, std);
+}
+
+static int camif_g_std(struct file *file, void *fh, v4l2_std_id *std)
+{
+	struct camif_priv *priv = video_drvdata(file);
+
+	*std = priv->current_std;
+	return 0;
+}
+
+static int camif_s_std(struct file *file, void *fh, v4l2_std_id std)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+	int ret;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	if (vb2_is_busy(&priv->buffer_queue))
+		return -EBUSY;
+
+	ret = v4l2_subdev_call(sensor->sd, video, s_std, std);
+	if (ret < 0)
+		return ret;
+
+	priv->current_std = std;
+	return 0;
+}
+
+static int camif_enum_input(struct file *file, void *fh,
+			    struct v4l2_input *input)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+	int index = input->index;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	if (index >= sensor->input.num)
+		return -EINVAL;
+
+	input->type = V4L2_INPUT_TYPE_CAMERA;
+	strncpy(input->name, sensor->input.name[index], sizeof(input->name));
+
+	if (index == priv->current_input) {
+		v4l2_subdev_call(sensor->sd, video, g_input_status,
+				 &input->status);
+		v4l2_subdev_call(sensor->sd, video, querystd, &input->std);
+	} else {
+		input->status = V4L2_IN_ST_NO_SIGNAL;
+		input->std = V4L2_STD_UNKNOWN;
+	}
+
+	return 0;
+}
+
+static int camif_g_input(struct file *file, void *fh, unsigned int *index)
+{
+	struct camif_priv *priv = video_drvdata(file);
+
+	*index = priv->current_input;
+	return 0;
+}
+
+static int camif_s_input(struct file *file, void *fh, unsigned int index)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+	int ret;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	if (index >= sensor->input.num)
+		return -EINVAL;
+
+	if (index == priv->current_input)
+		return 0;
+
+	/* select the sensor's input */
+	ret = v4l2_subdev_call(sensor->sd, video, s_routing,
+			       sensor->input.value[index], 0, 0);
+	if (!ret)
+		priv->current_input = index;
+
+	return ret;
+}
+
+static int camif_g_parm(struct file *file, void *fh,
+			struct v4l2_streamparm *a)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	return v4l2_subdev_call(sensor->sd, video, g_parm, a);
+}
+
+static int camif_s_parm(struct file *file, void *fh,
+			struct v4l2_streamparm *a)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct imx_media_subdev *sensor;
+
+	sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
+	if (IS_ERR(sensor)) {
+		v4l2_err(&priv->sd, "no sensor attached\n");
+		return PTR_ERR(sensor);
+	}
+
+	if (a->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	return v4l2_subdev_call(sensor->sd, video, s_parm, a);
+}
+
+static const struct v4l2_ioctl_ops camif_ioctl_ops = {
+	.vidioc_querycap	= vidioc_querycap,
+
+	.vidioc_enum_fmt_vid_cap        = camif_enum_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap           = camif_g_fmt_vid_cap,
+	.vidioc_try_fmt_vid_cap         = camif_try_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap           = camif_s_fmt_vid_cap,
+
+	.vidioc_querystd        = camif_querystd,
+	.vidioc_g_std           = camif_g_std,
+	.vidioc_s_std           = camif_s_std,
+
+	.vidioc_enum_input      = camif_enum_input,
+	.vidioc_g_input         = camif_g_input,
+	.vidioc_s_input         = camif_s_input,
+
+	.vidioc_g_parm          = camif_g_parm,
+	.vidioc_s_parm          = camif_s_parm,
+
+	.vidioc_reqbufs		= vb2_ioctl_reqbufs,
+	.vidioc_create_bufs     = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf     = vb2_ioctl_prepare_buf,
+	.vidioc_querybuf	= vb2_ioctl_querybuf,
+	.vidioc_qbuf		= vb2_ioctl_qbuf,
+	.vidioc_dqbuf		= vb2_ioctl_dqbuf,
+	.vidioc_expbuf		= vb2_ioctl_expbuf,
+	.vidioc_streamon	= vb2_ioctl_streamon,
+	.vidioc_streamoff	= vb2_ioctl_streamoff,
+};
+
+/*
+ * Queue operations
+ */
+
+static u32 camif_get_sizeimage(struct camif_priv *priv)
+{
+	struct v4l2_mbus_framefmt *outfmt;
+	const struct imx_media_pixfmt *outcc;
+
+	outfmt = &priv->format_mbus[priv->output_pad];
+	outcc = priv->cc[priv->output_pad];
+	return (outfmt->width * outfmt->height * outcc->bpp) >> 3;
+}
+
+static int camif_queue_setup(struct vb2_queue *vq,
+			     unsigned int *nbuffers, unsigned int *nplanes,
+			     unsigned int sizes[], struct device *alloc_devs[])
+{
+	struct camif_priv *priv = vb2_get_drv_priv(vq);
+	unsigned int count = *nbuffers;
+	u32 sizeimage;
+
+	if (!priv->src_sd)
+		return -EPIPE;
+
+	if (vq->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	sizeimage = camif_get_sizeimage(priv);
+
+	count = min_t(unsigned int, count, IMX_MEDIA_MAX_RING_BUFS);
+	count = max_t(unsigned int, count, IMX_MEDIA_MIN_RING_BUFS);
+
+	while (sizeimage * count > VID_MEM_LIMIT)
+		count--;
+
+	if (count < IMX_MEDIA_MIN_RING_BUFS)
+		return -EINVAL;
+
+	*nplanes = 1;
+	*nbuffers = count;
+	sizes[0] = sizeimage;
+
+	return 0;
+}
+
+static int camif_buf_init(struct vb2_buffer *vb)
+{
+	struct vb2_queue *vq = vb->vb2_queue;
+	struct camif_priv *priv = vb2_get_drv_priv(vq);
+	struct imx_media_buffer *buf = to_imx_media_vb(vb);
+
+	if (!priv->src_sd)
+		return -EPIPE;
+
+	INIT_LIST_HEAD(&buf->list);
+
+	return 0;
+}
+
+static int camif_buf_prepare(struct vb2_buffer *vb)
+{
+	struct vb2_queue *vq = vb->vb2_queue;
+	struct camif_priv *priv = vb2_get_drv_priv(vq);
+	u32 sizeimage = camif_get_sizeimage(priv);
+	int ret;
+
+	if (!priv->src_sd)
+		return -EPIPE;
+
+	if (vb2_plane_size(vb, 0) < sizeimage) {
+		v4l2_err(&priv->sd,
+			 "data will not fit into plane (%lu < %lu)\n",
+			 vb2_plane_size(vb, 0), (long)sizeimage);
+		return -EINVAL;
+	}
+
+	vb2_set_plane_payload(vb, 0, sizeimage);
+
+	if (!priv->in_ring) {
+		priv->in_ring = imx_media_alloc_dma_buf_ring(
+			priv->md, &priv->src_sd->entity, &priv->sd.entity,
+			sizeimage, vq->num_buffers, false);
+		if (IS_ERR(priv->in_ring)) {
+			v4l2_err(&priv->sd, "failed to alloc dma-buf ring\n");
+			ret = PTR_ERR(priv->in_ring);
+			priv->in_ring = NULL;
+			return ret;
+		}
+	}
+
+	ret = imx_media_dma_buf_queue_from_vb(priv->in_ring, vb);
+	if (ret)
+		goto free_ring;
+
+	return 0;
+
+free_ring:
+	imx_media_free_dma_buf_ring(priv->in_ring);
+	priv->in_ring = NULL;
+	return ret;
+}
+
+static void camif_buf_queue(struct vb2_buffer *vb)
+{
+	struct camif_priv *priv = vb2_get_drv_priv(vb->vb2_queue);
+	struct imx_media_buffer *buf = to_imx_media_vb(vb);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->q_lock, flags);
+
+	list_add_tail(&buf->list, &priv->ready_q);
+
+	spin_unlock_irqrestore(&priv->q_lock, flags);
+}
+
+static int camif_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct camif_priv *priv = vb2_get_drv_priv(vq);
+	u32 sizeimage = camif_get_sizeimage(priv);
+	struct imx_media_buffer *buf, *tmp;
+	unsigned long flags;
+	int ret;
+
+	if (vb2_is_streaming(vq))
+		return 0;
+
+	if (!priv->src_sd)
+		return -EPIPE;
+
+	if (!priv->in_ring) {
+		priv->in_ring = imx_media_alloc_dma_buf_ring(
+			priv->md, &priv->src_sd->entity, &priv->sd.entity,
+			sizeimage, vq->num_buffers, false);
+		if (IS_ERR(priv->in_ring)) {
+			v4l2_err(&priv->sd, "failed to alloc dma-buf ring\n");
+			ret = PTR_ERR(priv->in_ring);
+			priv->in_ring = NULL;
+			goto return_bufs;
+		}
+	}
+
+	ret = imx_media_pipeline_set_stream(priv->md, &priv->sd.entity,
+					    &priv->mp, true);
+	if (ret) {
+		v4l2_err(&priv->sd, "pipeline_set_stream failed with %d\n",
+			 ret);
+		goto free_ring;
+	}
+
+	priv->stop = false;
+
+	return 0;
+
+free_ring:
+	imx_media_free_dma_buf_ring(priv->in_ring);
+	priv->in_ring = NULL;
+return_bufs:
+	spin_lock_irqsave(&priv->q_lock, flags);
+	list_for_each_entry_safe(buf, tmp, &priv->ready_q, list) {
+		list_del(&buf->list);
+		vb2_buffer_done(&buf->vbuf.vb2_buf, VB2_BUF_STATE_QUEUED);
+	}
+	spin_unlock_irqrestore(&priv->q_lock, flags);
+	return ret;
+}
+
+static void camif_stop_streaming(struct vb2_queue *vq)
+{
+	struct camif_priv *priv = vb2_get_drv_priv(vq);
+	struct imx_media_buffer *frame;
+	unsigned long flags;
+	int ret;
+
+	if (!vb2_is_streaming(vq))
+		return;
+
+	spin_lock_irqsave(&priv->q_lock, flags);
+	priv->stop = true;
+	spin_unlock_irqrestore(&priv->q_lock, flags);
+
+	ret = imx_media_pipeline_set_stream(priv->md, &priv->sd.entity,
+					    &priv->mp, false);
+	if (ret)
+		v4l2_warn(&priv->sd, "pipeline_set_stream failed with %d\n",
+			  ret);
+
+	if (priv->in_ring) {
+		v4l2_warn(&priv->sd, "%s: in_ring was not freed\n",
+			  __func__);
+		imx_media_free_dma_buf_ring(priv->in_ring);
+		priv->in_ring = NULL;
+	}
+
+	/* release all active buffers */
+	spin_lock_irqsave(&priv->q_lock, flags);
+	while (!list_empty(&priv->ready_q)) {
+		frame = list_entry(priv->ready_q.next,
+				   struct imx_media_buffer, list);
+		list_del(&frame->list);
+		vb2_buffer_done(&frame->vbuf.vb2_buf, VB2_BUF_STATE_ERROR);
+	}
+	spin_unlock_irqrestore(&priv->q_lock, flags);
+}
+
+static struct vb2_ops camif_qops = {
+	.queue_setup	 = camif_queue_setup,
+	.buf_init        = camif_buf_init,
+	.buf_prepare	 = camif_buf_prepare,
+	.buf_queue	 = camif_buf_queue,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
+	.start_streaming = camif_start_streaming,
+	.stop_streaming  = camif_stop_streaming,
+};
+
+/*
+ * File operations
+ */
+static int camif_open(struct file *file)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	int ret;
+
+	if (mutex_lock_interruptible(&priv->mutex))
+		return -ERESTARTSYS;
+
+	ret = v4l2_fh_open(file);
+	if (ret)
+		v4l2_err(&priv->sd, "v4l2_fh_open failed\n");
+
+	mutex_unlock(&priv->mutex);
+	return ret;
+}
+
+static int camif_release(struct file *file)
+{
+	struct camif_priv *priv = video_drvdata(file);
+	struct vb2_queue *vq = &priv->buffer_queue;
+	int ret = 0;
+
+	mutex_lock(&priv->mutex);
+
+	if (file->private_data == vq->owner) {
+		vb2_queue_release(vq);
+		vq->owner = NULL;
+	}
+
+	v4l2_fh_release(file);
+	mutex_unlock(&priv->mutex);
+	return ret;
+}
+
+static const struct v4l2_file_operations camif_fops = {
+	.owner		= THIS_MODULE,
+	.open		= camif_open,
+	.release	= camif_release,
+	.poll		= vb2_fop_poll,
+	.unlocked_ioctl	= video_ioctl2,
+	.mmap		= vb2_fop_mmap,
+};
+
+static struct video_device camif_videodev = {
+	.fops		= &camif_fops,
+	.ioctl_ops	= &camif_ioctl_ops,
+	.minor		= -1,
+	.release	= video_device_release,
+	.vfl_dir	= VFL_DIR_RX,
+	.tvnorms	= V4L2_STD_NTSC | V4L2_STD_PAL | V4L2_STD_SECAM,
+};
+
+/*
+ * Subdev and media entity operations
+ */
+
+static void camif_new_dma_buf(struct camif_priv *priv)
+{
+	struct imx_media_dma_buf *dmabuf;
+	struct imx_media_buffer *buf;
+	enum vb2_buffer_state state;
+	struct vb2_buffer *vb;
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->q_lock, flags);
+
+	if (priv->stop || list_empty(&priv->ready_q))
+		goto unlock;
+
+	dmabuf = imx_media_dma_buf_dequeue(priv->in_ring);
+	if (!dmabuf)
+		goto unlock;
+
+	vb = dmabuf->vb;
+	buf = to_imx_media_vb(vb);
+	if (list_empty(&buf->list)) {
+		dev_dbg(priv->dev, "%s: buf%d not queued\n", __func__,
+			vb->index);
+		goto unlock;
+	}
+
+	dev_dbg(priv->dev, "%s: new buf%d\n", __func__, vb->index);
+	vb->timestamp = ktime_get_ns();
+	list_del_init(&buf->list);
+	state = dmabuf->status == IMX_MEDIA_BUF_STATUS_DONE ?
+		VB2_BUF_STATE_DONE : VB2_BUF_STATE_ERROR;
+	vb2_buffer_done(vb, state);
+unlock:
+	spin_unlock_irqrestore(&priv->q_lock, flags);
+}
+
+static long camif_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
+{
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+	struct imx_media_dma_buf_ring **ring;
+
+	switch (cmd) {
+	case IMX_MEDIA_REQ_DMA_BUF_SINK_RING:
+		if (!priv->in_ring)
+			return -EINVAL;
+		ring = (struct imx_media_dma_buf_ring **)arg;
+		*ring = priv->in_ring;
+		break;
+	case IMX_MEDIA_NEW_DMA_BUF:
+		camif_new_dma_buf(priv);
+		break;
+	case IMX_MEDIA_REL_DMA_BUF_SINK_RING:
+		/* src indicates sink buffer ring can be freed */
+		if (!priv->in_ring)
+			return 0;
+		v4l2_info(sd, "%s: freeing sink ring\n", __func__);
+		imx_media_free_dma_buf_ring(priv->in_ring);
+		priv->in_ring = NULL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int camif_link_setup(struct media_entity *entity,
+			    const struct media_pad *local,
+			    const struct media_pad *remote, u32 flags)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+	struct video_device *vfd = &priv->vfd;
+	struct v4l2_subdev *remote_sd;
+	int ret = 0;
+
+	dev_dbg(priv->dev, "link setup %s -> %s", remote->entity->name,
+		local->entity->name);
+
+	if (is_media_entity_v4l2_video_device(remote->entity))
+		return 0;
+
+	WARN_ON(local->flags & MEDIA_PAD_FL_SOURCE);
+
+	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
+
+	/* reset controls to refresh with inherited from subdevs */
+	v4l2_ctrl_handler_free(vfd->ctrl_handler);
+	v4l2_ctrl_handler_init(vfd->ctrl_handler, 0);
+
+	if (flags & MEDIA_LNK_FL_ENABLED) {
+		if (priv->src_sd)
+			return -EBUSY;
+		priv->src_sd = remote_sd;
+		ret = imx_media_inherit_controls(priv->md, vfd,
+						 &priv->src_sd->entity);
+	} else {
+		priv->src_sd = NULL;
+	}
+
+	return ret;
+}
+
+static int camif_enum_mbus_code(struct v4l2_subdev *sd,
+				struct v4l2_subdev_pad_config *cfg,
+				struct v4l2_subdev_mbus_code_enum *code)
+{
+	if (code->pad >= CAMIF_NUM_PADS)
+		return -EINVAL;
+
+	return imx_media_enum_format(&code->code, code->index, true, true);
+}
+
+static int camif_get_fmt(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg,
+			 struct v4l2_subdev_format *sdformat)
+{
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+
+	if (sdformat->pad >= CAMIF_NUM_PADS)
+		return -EINVAL;
+
+	sdformat->format = priv->format_mbus[sdformat->pad];
+
+	return 0;
+}
+
+static int camif_set_fmt(struct v4l2_subdev *sd,
+			 struct v4l2_subdev_pad_config *cfg,
+			 struct v4l2_subdev_format *sdformat)
+{
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+	const struct imx_media_pixfmt *cc;
+	u32 code;
+
+	if (sdformat->pad >= CAMIF_NUM_PADS)
+		return -EINVAL;
+
+	if (vb2_is_busy(&priv->buffer_queue)) {
+		v4l2_err(&priv->sd, "%s queue busy\n", __func__);
+		return -EBUSY;
+	}
+
+	cc = imx_media_find_format(0, sdformat->format.code, true, true);
+	if (!cc) {
+		imx_media_enum_format(&code, 0, true, true);
+		cc = imx_media_find_format(0, code, true, true);
+		sdformat->format.code = cc->codes[0];
+	}
+
+	/* Output pad mirrors input pad, no limitations on input pads */
+	if (sdformat->pad == priv->output_pad)
+		sdformat->format = priv->format_mbus[priv->input_pad];
+
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY) {
+		cfg->try_fmt = sdformat->format;
+	} else {
+		priv->format_mbus[sdformat->pad] = sdformat->format;
+		priv->cc[sdformat->pad] = cc;
+	}
+
+	return 0;
+}
+
+static int camif_init_pads(struct camif_priv *priv)
+{
+	struct video_device *vfd = &priv->vfd;
+	struct imx_media_subdev *imxsd;
+	struct imx_media_pad *pad;
+	int i, ret;
+
+	imxsd = imx_media_find_subdev_by_sd(priv->md, &priv->sd);
+	if (IS_ERR(imxsd))
+		return PTR_ERR(imxsd);
+
+	if (imxsd->num_sink_pads != 1 || imxsd->num_src_pads != 1) {
+		v4l2_err(&priv->sd, "invalid num pads %d/%d\n",
+			 imxsd->num_sink_pads, imxsd->num_src_pads);
+		return -EINVAL;
+	}
+
+	priv->vd_pad.flags = MEDIA_PAD_FL_SINK;
+	ret = media_entity_pads_init(&vfd->entity, 1, &priv->vd_pad);
+	if (ret) {
+		v4l2_err(&priv->sd, "failed to init device node pad\n");
+		return ret;
+	}
+
+	for (i = 0; i < CAMIF_NUM_PADS; i++) {
+		pad = &imxsd->pad[i];
+		priv->pad[i] = pad->pad;
+		if (priv->pad[i].flags & MEDIA_PAD_FL_SINK)
+			priv->input_pad = i;
+		else
+			priv->output_pad = i;
+
+		/* set a default mbus format  */
+		ret = imx_media_init_mbus_fmt(&priv->format_mbus[i],
+					      640, 480, 0, V4L2_FIELD_NONE,
+					      &priv->cc[i]);
+		if (ret)
+			return ret;
+	}
+
+	return media_entity_pads_init(&priv->sd.entity, CAMIF_NUM_PADS,
+				      priv->pad);
+}
+
+static int camif_registered(struct v4l2_subdev *sd)
+{
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+	struct vb2_queue *vq = &priv->buffer_queue;
+	struct video_device *vfd = &priv->vfd;
+	struct v4l2_mbus_framefmt *infmt, *outfmt;
+	int ret;
+
+	/* get media device */
+	priv->md = dev_get_drvdata(sd->v4l2_dev->dev);
+
+	vfd->v4l2_dev = sd->v4l2_dev;
+
+	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
+	if (ret) {
+		v4l2_err(sd, "Failed to register video device\n");
+		return ret;
+	}
+
+	vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vq->io_modes = VB2_MMAP | VB2_DMABUF;
+	vq->drv_priv = priv;
+	vq->buf_struct_size = sizeof(struct imx_media_buffer);
+	vq->ops = &camif_qops;
+	vq->mem_ops = &vb2_dma_contig_memops;
+	vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	vq->lock = &priv->mutex;
+	vq->min_buffers_needed = 2;
+	vq->dev = priv->dev;
+
+	ret = vb2_queue_init(vq);
+	if (ret) {
+		v4l2_err(sd, "vb2_queue_init failed\n");
+		goto unreg;
+	}
+
+	INIT_LIST_HEAD(&priv->ready_q);
+
+	ret = camif_init_pads(priv);
+	if (ret) {
+		v4l2_err(sd, "camif_init_pads failed\n");
+		goto unreg;
+	}
+
+	/* create the link to our device node */
+	ret = media_create_pad_link(&sd->entity, priv->output_pad,
+				    &vfd->entity, 0,
+				    MEDIA_LNK_FL_IMMUTABLE |
+				    MEDIA_LNK_FL_ENABLED);
+	if (ret) {
+		v4l2_err(sd, "failed to create link to device node\n");
+		goto unreg;
+	}
+
+	/* setup default pad formats */
+	infmt = &priv->format_mbus[priv->input_pad];
+	outfmt = &priv->format_mbus[priv->output_pad];
+	ret = imx_media_init_mbus_fmt(outfmt, 640, 480, 0, V4L2_FIELD_NONE,
+				      &priv->cc[priv->output_pad]);
+	if (ret)
+		goto unreg;
+
+	*infmt = *outfmt;
+	priv->cc[priv->input_pad] = priv->cc[priv->output_pad];
+
+	priv->current_std = V4L2_STD_UNKNOWN;
+
+	v4l2_info(sd, "Registered %s as /dev/%s\n", vfd->name,
+		  video_device_node_name(vfd));
+
+	vfd->ctrl_handler = &priv->ctrl_hdlr;
+
+	return 0;
+unreg:
+	video_unregister_device(vfd);
+	return ret;
+}
+
+static void camif_unregistered(struct v4l2_subdev *sd)
+{
+	struct camif_priv *priv = v4l2_get_subdevdata(sd);
+	struct video_device *vfd = &priv->vfd;
+
+	mutex_lock(&priv->mutex);
+
+	if (video_is_registered(vfd)) {
+		video_unregister_device(vfd);
+		media_entity_cleanup(&vfd->entity);
+	}
+
+	mutex_unlock(&priv->mutex);
+}
+
+static const struct v4l2_subdev_internal_ops camif_internal_ops = {
+	.registered = camif_registered,
+	.unregistered = camif_unregistered,
+};
+
+static struct v4l2_subdev_core_ops camif_core_ops = {
+	.ioctl = camif_ioctl,
+};
+
+static struct media_entity_operations camif_entity_ops = {
+	.link_setup = camif_link_setup,
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static struct v4l2_subdev_pad_ops camif_pad_ops = {
+	.enum_mbus_code = camif_enum_mbus_code,
+	.get_fmt = camif_get_fmt,
+	.set_fmt = camif_set_fmt,
+};
+
+static struct v4l2_subdev_ops camif_subdev_ops = {
+	.pad = &camif_pad_ops,
+	.core = &camif_core_ops,
+};
+
+static int camif_probe(struct platform_device *pdev)
+{
+	struct imx_media_internal_sd_platformdata *pdata;
+	struct camif_priv *priv;
+	struct video_device *vfd;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+	priv->dev = &pdev->dev;
+
+	pdata = priv->dev->platform_data;
+
+	mutex_init(&priv->mutex);
+	spin_lock_init(&priv->q_lock);
+
+	v4l2_subdev_init(&priv->sd, &camif_subdev_ops);
+	v4l2_set_subdevdata(&priv->sd, priv);
+	priv->sd.internal_ops = &camif_internal_ops;
+	priv->sd.entity.ops = &camif_entity_ops;
+	priv->sd.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
+	priv->sd.dev = &pdev->dev;
+	priv->sd.owner = THIS_MODULE;
+	/* get our group id and camif id */
+	priv->sd.grp_id = pdata->grp_id;
+	priv->id = (pdata->grp_id >> IMX_MEDIA_GRP_ID_CAMIF_BIT) - 1;
+	strncpy(priv->sd.name, pdata->sd_name, sizeof(priv->sd.name));
+	snprintf(camif_videodev.name, sizeof(camif_videodev.name),
+		 "%s devnode", pdata->sd_name);
+
+	priv->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE | V4L2_SUBDEV_FL_HAS_EVENTS;
+
+	vfd = &priv->vfd;
+	*vfd = camif_videodev;
+	vfd->lock = &priv->mutex;
+	vfd->queue = &priv->buffer_queue;
+
+	video_set_drvdata(vfd, priv);
+
+	v4l2_ctrl_handler_init(&priv->ctrl_hdlr, 0);
+
+	ret = v4l2_async_register_subdev(&priv->sd);
+	if (ret)
+		v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
+
+	return ret;
+}
+
+static int camif_remove(struct platform_device *pdev)
+{
+	struct camif_priv *priv =
+		(struct camif_priv *)platform_get_drvdata(pdev);
+
+	v4l2_ctrl_handler_free(&priv->ctrl_hdlr);
+	v4l2_async_unregister_subdev(&priv->sd);
+	media_entity_cleanup(&priv->sd.entity);
+	v4l2_device_unregister_subdev(&priv->sd);
+
+	return 0;
+}
+
+static const struct platform_device_id camif_ids[] = {
+	{ .name = "imx-media-camif" },
+	{ },
+};
+MODULE_DEVICE_TABLE(platform, camif_ids);
+
+static struct platform_driver imx_camif_driver = {
+	.probe		= camif_probe,
+	.remove		= camif_remove,
+	.driver		= {
+		.name	= "imx-media-camif",
+	},
+};
+
+module_platform_driver(imx_camif_driver);
+
+MODULE_DESCRIPTION("i.MX camera interface subdev driver");
+MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1553563 — [PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-07 03:20 +0100
Subject[PATCH v3 21/24] media: imx: Add MIPI CSI-2 Receiver subdev driver
Message-ID<sWOLU-2qI-53@gated-at.bofh.it>
In reply to#1553560
Adds MIPI CSI-2 Receiver subdev driver. This subdev is required
for sensors with a MIPI CSI2 interface.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 drivers/staging/media/imx/Makefile        |   1 +
 drivers/staging/media/imx/imx-mipi-csi2.c | 501 ++++++++++++++++++++++++++++++
 2 files changed, 502 insertions(+)
 create mode 100644 drivers/staging/media/imx/imx-mipi-csi2.c

diff --git a/drivers/staging/media/imx/Makefile b/drivers/staging/media/imx/Makefile
index fe9e992..0decef7 100644
--- a/drivers/staging/media/imx/Makefile
+++ b/drivers/staging/media/imx/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_VIDEO_IMX_MEDIA) += imx-ic.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-csi.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-smfc.o
 obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-camif.o
+obj-$(CONFIG_VIDEO_IMX_CAMERA) += imx-mipi-csi2.o
diff --git a/drivers/staging/media/imx/imx-mipi-csi2.c b/drivers/staging/media/imx/imx-mipi-csi2.c
new file mode 100644
index 0000000..daa6e1d
--- /dev/null
+++ b/drivers/staging/media/imx/imx-mipi-csi2.c
@@ -0,0 +1,501 @@
+/*
+ * MIPI CSI-2 Receiver Subdev for Freescale i.MX5/6 SOC.
+ *
+ * Copyright (c) 2012-2014 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.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-of.h>
+#include <media/v4l2-subdev.h>
+#include <video/imx-ipu-v3.h>
+#include "imx-media.h"
+
+/*
+ * there must be 5 pads: 1 input pad from sensor, and
+ * the 4 virtual channel output pads
+ */
+#define CSI2_NUM_SINK_PADS  1
+#define CSI2_NUM_SRC_PADS   4
+#define CSI2_NUM_PADS       5
+
+struct imxcsi2_dev {
+	struct device          *dev;
+	struct imx_media_dev   *md;
+	struct v4l2_subdev      sd;
+	struct media_pad       pad[CSI2_NUM_PADS];
+	struct v4l2_mbus_framefmt format_mbus;
+	struct v4l2_subdev     *src_sd;
+	struct v4l2_subdev     *sink_sd[CSI2_NUM_SRC_PADS];
+	int                    input_pad;
+	struct clk             *dphy_clk;
+	struct clk             *cfg_clk;
+	struct clk             *pix_clk; /* what is this? */
+	void __iomem           *base;
+	int                     intr1;
+	int                     intr2;
+	struct v4l2_of_bus_mipi_csi2 bus;
+	bool                    on;
+	bool                    stream_on;
+};
+
+#define DEVICE_NAME "imx6-mipi-csi2"
+
+/* Register offsets */
+#define CSI2_VERSION            0x000
+#define CSI2_N_LANES            0x004
+#define CSI2_PHY_SHUTDOWNZ      0x008
+#define CSI2_DPHY_RSTZ          0x00c
+#define CSI2_RESETN             0x010
+#define CSI2_PHY_STATE          0x014
+#define CSI2_DATA_IDS_1         0x018
+#define CSI2_DATA_IDS_2         0x01c
+#define CSI2_ERR1               0x020
+#define CSI2_ERR2               0x024
+#define CSI2_MSK1               0x028
+#define CSI2_MSK2               0x02c
+#define CSI2_PHY_TST_CTRL0      0x030
+#define CSI2_PHY_TST_CTRL1      0x034
+#define CSI2_SFT_RESET          0xf00
+
+static inline struct imxcsi2_dev *sd_to_dev(struct v4l2_subdev *sdev)
+{
+	return container_of(sdev, struct imxcsi2_dev, sd);
+}
+
+static inline u32 imxcsi2_read(struct imxcsi2_dev *csi2, unsigned int regoff)
+{
+	return readl(csi2->base + regoff);
+}
+
+static inline void imxcsi2_write(struct imxcsi2_dev *csi2, u32 val,
+				 unsigned int regoff)
+{
+	writel(val, csi2->base + regoff);
+}
+
+static void imxcsi2_set_lanes(struct imxcsi2_dev *csi2)
+{
+	int lanes = csi2->bus.num_data_lanes;
+
+	imxcsi2_write(csi2, lanes - 1, CSI2_N_LANES);
+}
+
+static void imxcsi2_enable(struct imxcsi2_dev *csi2, bool enable)
+{
+	if (enable) {
+		imxcsi2_write(csi2, 0xffffffff, CSI2_PHY_SHUTDOWNZ);
+		imxcsi2_write(csi2, 0xffffffff, CSI2_DPHY_RSTZ);
+		imxcsi2_write(csi2, 0xffffffff, CSI2_RESETN);
+	} else {
+		imxcsi2_write(csi2, 0x0, CSI2_PHY_SHUTDOWNZ);
+		imxcsi2_write(csi2, 0x0, CSI2_DPHY_RSTZ);
+		imxcsi2_write(csi2, 0x0, CSI2_RESETN);
+	}
+}
+
+static void imxcsi2_reset(struct imxcsi2_dev *csi2)
+{
+	imxcsi2_enable(csi2, false);
+
+	imxcsi2_write(csi2, 0x00000001, CSI2_PHY_TST_CTRL0);
+	imxcsi2_write(csi2, 0x00000000, CSI2_PHY_TST_CTRL1);
+	imxcsi2_write(csi2, 0x00000000, CSI2_PHY_TST_CTRL0);
+	imxcsi2_write(csi2, 0x00000002, CSI2_PHY_TST_CTRL0);
+	imxcsi2_write(csi2, 0x00010044, CSI2_PHY_TST_CTRL1);
+	imxcsi2_write(csi2, 0x00000000, CSI2_PHY_TST_CTRL0);
+	imxcsi2_write(csi2, 0x00000014, CSI2_PHY_TST_CTRL1);
+	imxcsi2_write(csi2, 0x00000002, CSI2_PHY_TST_CTRL0);
+	imxcsi2_write(csi2, 0x00000000, CSI2_PHY_TST_CTRL0);
+
+	imxcsi2_enable(csi2, true);
+}
+
+static int imxcsi2_dphy_wait(struct imxcsi2_dev *csi2)
+{
+	u32 reg;
+	int i;
+
+	/* wait for mipi sensor ready */
+	for (i = 0; i < 50; i++) {
+		reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
+		if (reg != 0x200)
+			break;
+		usleep_range(10000, 20000);
+	}
+
+	if (i >= 50) {
+		v4l2_err(&csi2->sd,
+			 "wait for clock lane timeout, phy_state = 0x%08x\n",
+			 reg);
+		return -ETIME;
+	}
+
+	/* wait for mipi stable */
+	for (i = 0; i < 50; i++) {
+		reg = imxcsi2_read(csi2, CSI2_ERR1);
+		if (reg == 0x0)
+			break;
+		usleep_range(10000, 20000);
+	}
+
+	if (i >= 50) {
+		v4l2_err(&csi2->sd,
+			 "wait for controller timeout, err1 = 0x%08x\n",
+			 reg);
+		return -ETIME;
+	}
+
+	/* finally let's wait for active clock on the clock lane */
+	for (i = 0; i < 50; i++) {
+		reg = imxcsi2_read(csi2, CSI2_PHY_STATE);
+		if (reg & (1 << 8))
+			break;
+		usleep_range(10000, 20000);
+	}
+
+	if (i >= 50) {
+		v4l2_err(&csi2->sd,
+			 "wait for active clock timeout, phy_state = 0x%08x\n",
+			 reg);
+		return -ETIME;
+	}
+
+	v4l2_info(&csi2->sd, "ready, dphy version 0x%x\n",
+		  imxcsi2_read(csi2, CSI2_VERSION));
+
+	return 0;
+}
+
+/*
+ * V4L2 subdev operations
+ */
+
+static int imxcsi2_link_setup(struct media_entity *entity,
+			      const struct media_pad *local,
+			      const struct media_pad *remote, u32 flags)
+{
+	struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+	struct v4l2_subdev *remote_sd;
+
+	dev_dbg(csi2->dev, "link setup %s -> %s", remote->entity->name,
+		local->entity->name);
+
+	remote_sd = media_entity_to_v4l2_subdev(remote->entity);
+
+	if (local->flags & MEDIA_PAD_FL_SOURCE) {
+		if (flags & MEDIA_LNK_FL_ENABLED) {
+			if (csi2->sink_sd[local->index])
+				return -EBUSY;
+			csi2->sink_sd[local->index] = remote_sd;
+		} else {
+			csi2->sink_sd[local->index] = NULL;
+		}
+	} else {
+		if (flags & MEDIA_LNK_FL_ENABLED) {
+			if (csi2->src_sd)
+				return -EBUSY;
+			csi2->src_sd = remote_sd;
+		} else {
+			csi2->src_sd = NULL;
+		}
+	}
+
+	return 0;
+}
+
+static int imxcsi2_s_power(struct v4l2_subdev *sd, int on)
+{
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+
+	if (on && !csi2->on) {
+		v4l2_info(&csi2->sd, "power ON\n");
+		clk_prepare_enable(csi2->cfg_clk);
+		clk_prepare_enable(csi2->dphy_clk);
+		imxcsi2_set_lanes(csi2);
+		imxcsi2_reset(csi2);
+	} else if (!on && csi2->on) {
+		v4l2_info(&csi2->sd, "power OFF\n");
+		imxcsi2_enable(csi2, false);
+		clk_disable_unprepare(csi2->dphy_clk);
+		clk_disable_unprepare(csi2->cfg_clk);
+	}
+
+	csi2->on = on;
+	return 0;
+}
+
+static int imxcsi2_s_stream(struct v4l2_subdev *sd, int enable)
+{
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+	int i, ret = 0;
+
+	if (!csi2->src_sd)
+		return -EPIPE;
+	for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
+		if (csi2->sink_sd[i])
+			break;
+	}
+	if (i >= CSI2_NUM_SRC_PADS)
+		return -EPIPE;
+
+	v4l2_info(sd, "stream %s\n", enable ? "ON" : "OFF");
+
+	if (enable && !csi2->stream_on) {
+		ret = clk_prepare_enable(csi2->pix_clk);
+		if (ret)
+			return ret;
+
+		ret = imxcsi2_dphy_wait(csi2);
+		if (ret) {
+			clk_disable_unprepare(csi2->pix_clk);
+			return ret;
+		}
+	} else if (!enable && csi2->stream_on) {
+		clk_disable_unprepare(csi2->pix_clk);
+	}
+
+	csi2->stream_on = enable;
+	return 0;
+}
+
+static int imxcsi2_get_fmt(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_pad_config *cfg,
+			   struct v4l2_subdev_format *sdformat)
+{
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+
+	sdformat->format = csi2->format_mbus;
+
+	return 0;
+}
+
+static int imxcsi2_set_fmt(struct v4l2_subdev *sd,
+			   struct v4l2_subdev_pad_config *cfg,
+			   struct v4l2_subdev_format *sdformat)
+{
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+
+	if (sdformat->pad >= CSI2_NUM_PADS)
+		return -EINVAL;
+
+	if (csi2->stream_on)
+		return -EBUSY;
+
+	/* Output pads mirror active input pad, no limits on input pads */
+	if (sdformat->pad != csi2->input_pad)
+		sdformat->format = csi2->format_mbus;
+
+	if (sdformat->which == V4L2_SUBDEV_FORMAT_TRY)
+		cfg->try_fmt = sdformat->format;
+	else
+		csi2->format_mbus = sdformat->format;
+
+	return 0;
+}
+
+/*
+ * retrieve our pads parsed from the OF graph by the media device
+ */
+static int imxcsi2_registered(struct v4l2_subdev *sd)
+{
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+	struct imx_media_subdev *imxsd;
+	struct imx_media_pad *pad;
+	int i, ret;
+
+	/* get media device */
+	csi2->md = dev_get_drvdata(sd->v4l2_dev->dev);
+
+	imxsd = imx_media_find_subdev_by_sd(csi2->md, sd);
+	if (IS_ERR(imxsd))
+		return PTR_ERR(imxsd);
+
+	if (imxsd->num_sink_pads != 1 || imxsd->num_src_pads != 4)
+		return -EINVAL;
+
+	for (i = 0; i < CSI2_NUM_PADS; i++) {
+		pad = &imxsd->pad[i];
+		csi2->pad[i] = pad->pad;
+		if (csi2->pad[i].flags & MEDIA_PAD_FL_SINK)
+			csi2->input_pad = i;
+	}
+
+	/* set a default mbus format  */
+	ret = imx_media_init_mbus_fmt(&csi2->format_mbus,
+				      640, 480, 0, V4L2_FIELD_NONE, NULL);
+	if (ret)
+		return ret;
+
+	return media_entity_pads_init(&sd->entity, CSI2_NUM_PADS, csi2->pad);
+}
+
+static struct media_entity_operations imxcsi2_entity_ops = {
+	.link_setup = imxcsi2_link_setup,
+	.link_validate = v4l2_subdev_link_validate,
+};
+
+static struct v4l2_subdev_core_ops imxcsi2_core_ops = {
+	.s_power = imxcsi2_s_power,
+};
+
+static struct v4l2_subdev_video_ops imxcsi2_video_ops = {
+	.s_stream = imxcsi2_s_stream,
+};
+
+static struct v4l2_subdev_pad_ops imxcsi2_pad_ops = {
+	.get_fmt = imxcsi2_get_fmt,
+	.set_fmt = imxcsi2_set_fmt,
+};
+
+static struct v4l2_subdev_ops imxcsi2_subdev_ops = {
+	.core = &imxcsi2_core_ops,
+	.video = &imxcsi2_video_ops,
+	.pad = &imxcsi2_pad_ops,
+};
+
+static struct v4l2_subdev_internal_ops imxcsi2_internal_ops = {
+	.registered = imxcsi2_registered,
+};
+
+static int imxcsi2_parse_endpoints(struct imxcsi2_dev *csi2)
+{
+	struct device_node *node = csi2->dev->of_node;
+	struct device_node *epnode;
+	struct v4l2_of_endpoint ep;
+
+	epnode = of_graph_get_next_endpoint(node, NULL);
+	if (!epnode) {
+		v4l2_err(&csi2->sd, "failed to get endpoint node\n");
+		return -EINVAL;
+	}
+
+	v4l2_of_parse_endpoint(epnode, &ep);
+	of_node_put(epnode);
+
+	if (ep.bus_type != V4L2_MBUS_CSI2) {
+		v4l2_err(&csi2->sd, "invalid bus type, must be MIPI CSI2\n");
+		return -EINVAL;
+	}
+
+	csi2->bus = ep.bus.mipi_csi2;
+
+	v4l2_info(&csi2->sd, "data lanes: %d\n", csi2->bus.num_data_lanes);
+	v4l2_info(&csi2->sd, "flags: 0x%08x\n", csi2->bus.flags);
+	return 0;
+}
+
+static int imxcsi2_probe(struct platform_device *pdev)
+{
+	struct imxcsi2_dev *csi2;
+	struct resource *res;
+	int ret;
+
+	csi2 = devm_kzalloc(&pdev->dev, sizeof(*csi2), GFP_KERNEL);
+	if (!csi2)
+		return -ENOMEM;
+
+	csi2->dev = &pdev->dev;
+
+	v4l2_subdev_init(&csi2->sd, &imxcsi2_subdev_ops);
+	v4l2_set_subdevdata(&csi2->sd, &pdev->dev);
+	csi2->sd.internal_ops = &imxcsi2_internal_ops;
+	csi2->sd.entity.ops = &imxcsi2_entity_ops;
+	csi2->sd.dev = &pdev->dev;
+	csi2->sd.owner = THIS_MODULE;
+	csi2->sd.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
+	strcpy(csi2->sd.name, DEVICE_NAME);
+	csi2->sd.entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+	csi2->sd.grp_id = IMX_MEDIA_GRP_ID_CSI2;
+
+	ret = imxcsi2_parse_endpoints(csi2);
+	if (ret)
+		return ret;
+
+	csi2->cfg_clk = devm_clk_get(&pdev->dev, "cfg");
+	if (IS_ERR(csi2->cfg_clk)) {
+		v4l2_err(&csi2->sd, "failed to get cfg clock\n");
+		ret = PTR_ERR(csi2->cfg_clk);
+		return ret;
+	}
+
+	csi2->dphy_clk = devm_clk_get(&pdev->dev, "dphy");
+	if (IS_ERR(csi2->dphy_clk)) {
+		v4l2_err(&csi2->sd, "failed to get dphy clock\n");
+		ret = PTR_ERR(csi2->dphy_clk);
+		return ret;
+	}
+
+	csi2->pix_clk = devm_clk_get(&pdev->dev, "pix");
+	if (IS_ERR(csi2->pix_clk)) {
+		v4l2_err(&csi2->sd, "failed to get pixel clock\n");
+		ret = PTR_ERR(csi2->pix_clk);
+		return ret;
+	}
+
+	csi2->intr1 = platform_get_irq(pdev, 0);
+	csi2->intr2 = platform_get_irq(pdev, 1);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (!res || csi2->intr1 < 0 || csi2->intr2 < 0) {
+		v4l2_err(&csi2->sd, "failed to get platform resources\n");
+		return -ENODEV;
+	}
+
+	csi2->base = devm_ioremap(&pdev->dev, res->start, PAGE_SIZE);
+	if (!csi2->base) {
+		v4l2_err(&csi2->sd, "failed to map CSI-2 registers\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(pdev, &csi2->sd);
+
+	return v4l2_async_register_subdev(&csi2->sd);
+}
+
+static int imxcsi2_remove(struct platform_device *pdev)
+{
+	struct v4l2_subdev *sd = platform_get_drvdata(pdev);
+	struct imxcsi2_dev *csi2 = sd_to_dev(sd);
+
+	imxcsi2_s_power(sd, 0);
+
+	v4l2_async_unregister_subdev(&csi2->sd);
+	media_entity_cleanup(&csi2->sd.entity);
+	v4l2_device_unregister_subdev(sd);
+
+	return 0;
+}
+
+static const struct of_device_id imxcsi2_dt_ids[] = {
+	{ .compatible = "fsl,imx6-mipi-csi2", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imxcsi2_dt_ids);
+
+static struct platform_driver imxcsi2_driver = {
+	.driver = {
+		.name = DEVICE_NAME,
+		.of_match_table = imxcsi2_dt_ids,
+	},
+	.probe = imxcsi2_probe,
+	.remove = imxcsi2_remove,
+};
+
+module_platform_driver(imxcsi2_driver);
+
+MODULE_DESCRIPTION("i.MX5/6 MIPI CSI-2 Receiver driver");
+MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
+MODULE_LICENSE("GPL");
+
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1557008

FromTim Harvey <tharvey@gateworks.com>
Date2017-01-12 00:20 +0100
Message-ID<sYAlr-4Mp-19@gated-at.bofh.it>
In reply to#1553560
On Fri, Jan 6, 2017 at 6:11 PM, Steve Longerbeam <slongerbeam@gmail.com> wrote:
> In version 3:
>
> Changes suggested by Rob Herring <robh@kernel.org>:
>
>   - prepended FIM node properties with vendor prefix "fsl,".
>
>   - make mipi csi-2 receiver compatible string SoC specific:
>     "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
>
>   - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
>
>   - removed board-specific info from the media driver binding doc. These
>     were all related to sensor bindings, which already are (adv7180)
>     or will be (ov564x) covered in separate binding docs. All reference
>     board info not related to DT bindings has been moved to
>     Documentation/media/v4l-drivers/imx.rst.
>
>   - removed "_mipi" from the OV5640 compatible string.
>
> Changes suggested by Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>:
>
>   Mostly cosmetic/non-functional changes which I won't list here, except
>   for the following:
>
>   - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
>
>   - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
>
>   - check/handle return code from required reg property of CSI port nodes.
>
>   - check/handle return code from clk_prepare_enable().
>
> Changes suggested by Fabio Estevam <festevam@gmail.com>:
>
>   - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
>
>   - finally got around to passing valid IOMUX pin config values to the
>     pin groups.
>
> Other changes:
>
>   - removed the FIM properties that overrided the v4l2 FIM control defaults
>     values. This was left-over from a requirement of a customer and is not
>     necessary here.
>
>   - The FIM must be explicitly enabled in the fim child node under the CSI
>     port nodes, using the status property. If not enabled, FIM v4l2 controls
>     will not appear in the video capture driver.
>
>   - brought in additional media types patch from Philipp Zabel. Use new
>     MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
>
>   - brought in latest platform generic video multiplexer subdevice driver
>     from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
>
>   - removed imx-media-of.h, moved those prototypes into imx-media.h.
>
>
<snip>

Hi Steve,

I took a stab at testing this today on a gw51xx which has an adv7180
hooked up as follows:
- i2c3@0x20
- 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
(CSI0_IPU1)
- PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
- IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
- all three analog inputs available to off-board connector

My patch to the imx6qdl-gw51xx dtsi is:
diff --git a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
index afec2c7..2583d72 100644
--- a/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-gw51xx.dtsi
@@ -165,6 +174,52 @@
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_i2c3>;
        status = "okay";
+
+       camera: adv7180@20 {
+               compatible = "adi,adv7180";
+               pinctrl-names = "default";
+               pinctrl-0 = <&pinctrl_adv7180>;
+               reg = <0x20>;
+               powerdown-gpio = <&gpio5 20 GPIO_ACTIVE_LOW>;
+               interrupt-parent = <&gpio5>;
+               interrupts = <23 GPIO_ACTIVE_LOW>;
+               inputs = <0x00 0x01 0x02>;
+               input-names = "ADV7180 Composite on Ain1",
+                             "ADV7180 Composite on Ain2",
+                             "ADV7180 Composite on Ain3";
+
+               port {
+                       adv7180_to_ipu1_csi0_mux: endpoint {
+                               remote-endpoint =
<&ipu1_csi0_mux_from_parallel_sensor>;
+                               bus-width = <8>;
+                       };
+               };
+       };
+};
+
+&ipu1_csi0_from_ipu1_csi0_mux {
+       bus-width = <8>;
+};
+
+&ipu1_csi0_mux_from_parallel_sensor {
+       remote-endpoint = <&adv7180_to_ipu1_csi0_mux>;
+       bus-width = <8>;
+};
+
+&ipu1_csi0 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&pinctrl_ipu1_csi0>;
+
+       /* enable frame interval monitor on this port */
+       fim {
+               status = "okay";
+       };
 };

 &pcie {
@@ -236,6 +291,13 @@

 &iomuxc {
        imx6qdl-gw51xx {
+               pinctrl_adv7180: adv7180grp {
+                       fsl,pins = <
+                               MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
 0x0001b0b0 /* VIDDEC_IRQ# */
+                               MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
 0x4001b0b0 /* VIDDEC_EN */
+                       >;
+               };
+
                pinctrl_enet: enetgrp {
                        fsl,pins = <
                                MX6QDL_PAD_RGMII_RXC__RGMII_RXC         0x1b030
@@ -306,6 +368,22 @@
                        >;
                };

+               pinctrl_ipu1_csi0: ipu1csi0grp { /* IPU1_CSI0: 8-bit input */
+                       fsl,pins = <
+
MX6QDL_PAD_CSI0_DAT12__IPU1_CSI0_DATA12    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT13__IPU1_CSI0_DATA13    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT14__IPU1_CSI0_DATA14    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT15__IPU1_CSI0_DATA15    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT16__IPU1_CSI0_DATA16    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT17__IPU1_CSI0_DATA17    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT18__IPU1_CSI0_DATA18    0x1b0b0
+
MX6QDL_PAD_CSI0_DAT19__IPU1_CSI0_DATA19    0x1b0b0
+                               MX6QDL_PAD_CSI0_MCLK__IPU1_CSI0_HSYNC
    0x1b0b0
+                               MX6QDL_PAD_CSI0_VSYNC__IPU1_CSI0_VSYNC
    0x1b0b0
+
MX6QDL_PAD_CSI0_PIXCLK__IPU1_CSI0_PIXCLK   0x1b0b0
+                       >;
+               };
+
                pinctrl_pcie: pciegrp {
                        fsl,pins = <
                                MX6QDL_PAD_GPIO_0__GPIO1_IO00           0x1b0b0

On an IMX6Q I'm getting the following when the adv7180 module loads:
[   12.862477] adv7180 2-0020: chip found @ 0x20 (21a8000.i2c)
[   12.907767] imx-media: Registered subdev adv7180 2-0020
[   12.907793] imx-media soc:media@0: Entity type for entity adv7180
2-0020 was not initialized!
[   12.907867] imx-media: imx_media_create_link: adv7180 2-0020:0 ->
ipu1_csi0_mux:1

Is the warning that adv7180 was not initialized expected and or an issue?

Now that your driver is hooking into the current media framework, I'm
not at all clear on how to link and configure the media entities.
Using 'media-ctl -p' to list the entities I see:
Media controller API version 0.1.0

Media device information
------------------------
driver          imx-media
model           imx-media
serial
bus info
hw revision     0x0
driver version  0.0.0

Device topology
- entity 1: ipu2_csi1_mux (3 pads, 1 link)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
        pad0: Sink
                [fmt:unknown/0x0]
        pad1: Sink
                [fmt:unknown/0x0]
        pad2: Source
                [fmt:unknown/0x0]
                -> "ipu2_csi1":0 []

- entity 5: ipu1_csi0_mux (3 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev1
        pad0: Sink
                [fmt:unknown/0x0]
        pad1: Sink
                [fmt:unknown/0x0]
                <- "adv7180 2-0020":0 []
        pad2: Source
                [fmt:unknown/0x0]
                -> "ipu1_csi0":0 []

- entity 9: ipu1_ic_prpenc (2 pads, 4 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev2
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_csi0":1 []
                <- "ipu1_csi1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif0":0 []
                -> "camif1":0 []

- entity 12: ipu1_ic_prpvf (2 pads, 8 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev3
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_csi0":1 []
                <- "ipu1_csi1":1 []
                <- "ipu1_smfc0":1 []
                <- "ipu1_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif0":0 []
                -> "camif1":0 []
                -> "ipu1_ic_pp0":0 []
                -> "ipu1_ic_pp1":0 []

- entity 15: ipu1_ic_pp0 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev4
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_ic_prpvf":1 []
                <- "ipu1_smfc0":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif0":0 []
                -> "camif1":0 []

- entity 18: ipu1_ic_pp1 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev5
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_ic_prpvf":1 []
                <- "ipu1_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif0":0 []
                -> "camif1":0 []

- entity 21: ipu2_ic_prpenc (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev6
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_csi0":1 []
                <- "ipu2_csi1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif2":0 []
                -> "camif3":0 []

- entity 24: ipu2_ic_prpvf (2 pads, 8 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev7
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_csi0":1 []
                <- "ipu2_csi1":1 []
                <- "ipu2_smfc0":1 []
                <- "ipu2_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif2":0 []
                -> "camif3":0 []
                -> "ipu2_ic_pp0":0 []
                -> "ipu2_ic_pp1":0 []

- entity 27: ipu2_ic_pp0 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev8
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_ic_prpvf":1 []
                <- "ipu2_smfc0":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif2":0 []
                -> "camif3":0 []

- entity 30: ipu2_ic_pp1 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev9
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_ic_prpvf":1 []
                <- "ipu2_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif2":0 []
                -> "camif3":0 []

- entity 33: ipu1_csi0 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev10
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_csi0_mux":2 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none
                 crop.bounds:(0,0)/640x480
                 crop:(0,0)/0x0]
                -> "ipu1_ic_prpenc":0 []
                -> "ipu1_ic_prpvf":0 []
                -> "ipu1_smfc0":0 []

- entity 36: ipu1_csi1 (2 pads, 3 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev11
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none
                 crop.bounds:(0,0)/640x480
                 crop:(0,0)/0x0]
                -> "ipu1_ic_prpenc":0 []
                -> "ipu1_ic_prpvf":0 []
                -> "ipu1_smfc1":0 []

- entity 39: ipu2_csi0 (2 pads, 3 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev12
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none
                 crop.bounds:(0,0)/640x480
                 crop:(0,0)/0x0]
                -> "ipu2_ic_prpenc":0 []
                -> "ipu2_ic_prpvf":0 []
                -> "ipu2_smfc0":0 []

- entity 42: ipu2_csi1 (2 pads, 4 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev13
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_csi1_mux":2 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none
                 crop.bounds:(0,0)/640x480
                 crop:(0,0)/0x0]
                -> "ipu2_ic_prpenc":0 []
                -> "ipu2_ic_prpvf":0 []
                -> "ipu2_smfc1":0 []

- entity 45: ipu1_smfc0 (2 pads, 5 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev14
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_csi0":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "ipu1_ic_prpvf":0 []
                -> "ipu1_ic_pp0":0 []
                -> "camif0":0 []
                -> "camif1":0 []

- entity 48: ipu1_smfc1 (2 pads, 5 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev15
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_csi1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "ipu1_ic_prpvf":0 []
                -> "ipu1_ic_pp1":0 []
                -> "camif0":0 []
                -> "camif1":0 []

- entity 51: ipu2_smfc0 (2 pads, 5 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev16
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_csi0":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "ipu2_ic_prpvf":0 []
                -> "ipu2_ic_pp0":0 []
                -> "camif2":0 []
                -> "camif3":0 []

- entity 54: ipu2_smfc1 (2 pads, 5 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev17
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_csi1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "ipu2_ic_prpvf":0 []
                -> "ipu2_ic_pp1":0 []
                -> "camif2":0 []
                -> "camif3":0 []

- entity 57: camif0 (2 pads, 7 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev18
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_ic_prpenc":1 []
                <- "ipu1_ic_prpvf":1 []
                <- "ipu1_ic_pp0":1 []
                <- "ipu1_ic_pp1":1 []
                <- "ipu1_smfc0":1 []
                <- "ipu1_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif0 devnode":0 [ENABLED,IMMUTABLE]

- entity 58: camif0 devnode (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video0
        pad0: Sink
                <- "camif0":1 [ENABLED,IMMUTABLE]

- entity 66: camif1 (2 pads, 7 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev19
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu1_ic_prpenc":1 []
                <- "ipu1_ic_prpvf":1 []
                <- "ipu1_ic_pp0":1 []
                <- "ipu1_ic_pp1":1 []
                <- "ipu1_smfc0":1 []
                <- "ipu1_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif1 devnode":0 [ENABLED,IMMUTABLE]

- entity 67: camif1 devnode (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video1
        pad0: Sink
                <- "camif1":1 [ENABLED,IMMUTABLE]

- entity 75: camif2 (2 pads, 7 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev20
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_ic_prpenc":1 []
                <- "ipu2_ic_prpvf":1 []
                <- "ipu2_ic_pp0":1 []
                <- "ipu2_ic_pp1":1 []
                <- "ipu2_smfc0":1 []
                <- "ipu2_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif2 devnode":0 [ENABLED,IMMUTABLE]

- entity 76: camif2 devnode (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video2
        pad0: Sink
                <- "camif2":1 [ENABLED,IMMUTABLE]

- entity 84: camif3 (2 pads, 7 links)
             type V4L2 subdev subtype Unknown flags 0
             device node name /dev/v4l-subdev21
        pad0: Sink
                [fmt:UYVY2X8/640x480 field:none]
                <- "ipu2_ic_prpenc":1 []
                <- "ipu2_ic_prpvf":1 []
                <- "ipu2_ic_pp0":1 []
                <- "ipu2_ic_pp1":1 []
                <- "ipu2_smfc0":1 []
                <- "ipu2_smfc1":1 []
        pad1: Source
                [fmt:UYVY2X8/640x480 field:none]
                -> "camif3 devnode":0 [ENABLED,IMMUTABLE]

- entity 85: camif3 devnode (1 pad, 1 link)
             type Node subtype V4L flags 0
             device node name /dev/video3
        pad0: Sink
                <- "camif3":1 [ENABLED,IMMUTABLE]

- entity 93: adv7180 2-0020 (1 pad, 1 link)
             type V4L2 subdev subtype Unknown flags 20004
             device node name /dev/v4l-subdev22
        pad0: Source
                [fmt:UYVY2X8/720x480 field:interlaced]
                -> "ipu1_csi0_mux":1 []

How do I link the entities here to be able to capture frames with
v4l2-ctl or gstreamer?

Additionally I've found that on an IMX6S/IMX6DL we crash while
registering the media-ic subdev's:
[    3.975473] imx-media: Registered subdev ipu1_csi1_mux
[    3.980921] imx-media: Registered subdev ipu1_csi0_mux
[    4.003205] imx-media: Registered subdev ipu1_ic_prpenc
[    4.025373] imx-media: Registered subdev ipu1_ic_prpvf
[    4.037944] ------------[ cut here ]------------
[    4.042571] Kernel BUG at c06717dc [verbose debug info unavailable]
[    4.048845] Internal error: Oops - BUG: 0 [#1] SMP ARM
[    4.053990] Modules linked in:
[    4.057076] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
4.9.0-rc6-00524-g84dad6e-dirty #446
[    4.065260] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
...
[    4.296250] [<c0671780>] (v4l2_subdev_init) from [<c06fb02c>]
(imx_ic_probe+0x94/0x1ac)
[    4.304271] [<c06faf98>] (imx_ic_probe) from [<c05173d8>]
(platform_drv_probe+0x54/0xb8)
[    4.312373]  r9:c0d5e858 r8:00000000 r7:fffffdfb r6:c0e5dbf8
r5:da603810 r4:c16738d8
[    4.320129] [<c0517384>] (platform_drv_probe) from [<c0515978>]
(driver_probe_device+0x20c/0x2c0)
[    4.329010]  r7:c0e5dbf8 r6:00000000 r5:da603810 r4:c16738d8
[    4.334681] [<c051576c>] (driver_probe_device) from [<c0515af4>]
(__driver_attach+0xc8/0xcc)
[    4.343129]  r9:c0d5e858 r8:00000000 r7:00000000 r6:da603844
r5:c0e5dbf8 r4:da603810
[    4.350889] [<c0515a2c>] (__driver_attach) from [<c0513adc>]
(bus_for_each_dev+0x74/0xa8)
[    4.359078]  r7:00000000 r6:c0515a2c r5:c0e5dbf8 r4:00000000
[    4.364753] [<c0513a68>] (bus_for_each_dev) from [<c05151d4>]
(driver_attach+0x20/0x28)

I assume there is an iteration that needs a test on a missing pointer
only available on chips with both IPU's or PRP

Regards,

Tim

[toc] | [prev] | [next] | [standalone]


#1557085

FromSteve Longerbeam <steve_longerbeam@mentor.com>
Date2017-01-12 04:30 +0100
Message-ID<sYEfn-7bu-1@gated-at.bofh.it>
In reply to#1557008
Hi Tim,


On 01/11/2017 03:14 PM, Tim Harvey wrote:
>
> <snip>
>
> Hi Steve,
>
> I took a stab at testing this today on a gw51xx which has an adv7180
> hooked up as follows:
> - i2c3@0x20
> - 8bit data bus from DAT12 to DAT19, HSYNC, VSYNC, PIXCLK on CSI0 pads
> (CSI0_IPU1)
> - PWRDWN# on MX6QDL_PAD_CSI0_DATA_EN__GPIO5_IO20
> - IRQ# on MX6QDL_PAD_CSI0_DAT5__GPIO5_IO23
> - all three analog inputs available to off-board connector
>
> My patch to the imx6qdl-gw51xx dtsi is:

As long as you used the patch to imx6qdl-sabreauto.dtsti that adds
the adv7180 support as a guide, you should be ok here.

> <snip>
>
>
>
> On an IMX6Q I'm getting the following when the adv7180 module loads:
> [   12.862477] adv7180 2-0020: chip found @ 0x20 (21a8000.i2c)
> [   12.907767] imx-media: Registered subdev adv7180 2-0020
> [   12.907793] imx-media soc:media@0: Entity type for entity adv7180
> 2-0020 was not initialized!
> [   12.907867] imx-media: imx_media_create_link: adv7180 2-0020:0 ->
> ipu1_csi0_mux:1
>
> Is the warning that adv7180 was not initialized expected and or an issue?

Yeah it's still a bug in the adv7180 driver, needs fixing.

>
> Now that your driver is hooking into the current media framework, I'm
> not at all clear on how to link and configure the media entities.

It's all documented at Documentation/media/v4l-drivers/imx.rst.
Follow the SabreAuto pipeline setup example.



> <snip>
>
>
> Additionally I've found that on an IMX6S/IMX6DL we crash while
> registering the media-ic subdev's:
> [    3.975473] imx-media: Registered subdev ipu1_csi1_mux
> [    3.980921] imx-media: Registered subdev ipu1_csi0_mux
> [    4.003205] imx-media: Registered subdev ipu1_ic_prpenc
> [    4.025373] imx-media: Registered subdev ipu1_ic_prpvf
> [    4.037944] ------------[ cut here ]------------
> [    4.042571] Kernel BUG at c06717dc [verbose debug info unavailable]
> [    4.048845] Internal error: Oops - BUG: 0 [#1] SMP ARM
> [    4.053990] Modules linked in:
> [    4.057076] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
> 4.9.0-rc6-00524-g84dad6e-dirty #446
> [    4.065260] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> ...
> [    4.296250] [<c0671780>] (v4l2_subdev_init) from [<c06fb02c>]
> (imx_ic_probe+0x94/0x1ac)
> [    4.304271] [<c06faf98>] (imx_ic_probe) from [<c05173d8>]
> (platform_drv_probe+0x54/0xb8)
> [    4.312373]  r9:c0d5e858 r8:00000000 r7:fffffdfb r6:c0e5dbf8
> r5:da603810 r4:c16738d8
> [    4.320129] [<c0517384>] (platform_drv_probe) from [<c0515978>]
> (driver_probe_device+0x20c/0x2c0)
> [    4.329010]  r7:c0e5dbf8 r6:00000000 r5:da603810 r4:c16738d8
> [    4.334681] [<c051576c>] (driver_probe_device) from [<c0515af4>]
> (__driver_attach+0xc8/0xcc)
> [    4.343129]  r9:c0d5e858 r8:00000000 r7:00000000 r6:da603844
> r5:c0e5dbf8 r4:da603810
> [    4.350889] [<c0515a2c>] (__driver_attach) from [<c0513adc>]
> (bus_for_each_dev+0x74/0xa8)
> [    4.359078]  r7:00000000 r6:c0515a2c r5:c0e5dbf8 r4:00000000
> [    4.364753] [<c0513a68>] (bus_for_each_dev) from [<c05151d4>]
> (driver_attach+0x20/0x28)
>
> I assume there is an iteration that needs a test on a missing pointer
> only available on chips with both IPU's or PRP

Yep, I only have quad boards here so I haven't gotten around to
testing on S/DL.

But it looks like I forgot to clear out the csi subdev pointer array before
passing it to imx_media_of_parse(). I think that might explain the OOPS
above. Try this patch:

diff --git a/drivers/staging/media/imx/imx-media-dev.c 
b/drivers/staging/media/imx/imx-media-dev.c
index 357654d..0cf2d61 100644
--- a/drivers/staging/media/imx/imx-media-dev.c
+++ b/drivers/staging/media/imx/imx-media-dev.c
@@ -379,7 +379,7 @@ static int imx_media_probe(struct platform_device *pdev)
  {
         struct device *dev = &pdev->dev;
         struct device_node *node = dev->of_node;
-       struct imx_media_subdev *csi[4];
+       struct imx_media_subdev *csi[4] = {0};
         struct imx_media_dev *imxmd;
         int ret;


Steve

[toc] | [prev] | [next] | [standalone]


#1558491 — Re: [PATCH v3 16/24] media: Add i.MX media core driver

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2017-01-13 16:30 +0100
SubjectRe: [PATCH v3 16/24] media: Add i.MX media core driver
Message-ID<sZbXJ-2vV-51@gated-at.bofh.it>
In reply to#1553560
Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
> Add the core media driver for i.MX SOC.
> 
> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
> ---
>  Documentation/media/v4l-drivers/imx.rst           | 443 ++++++++++
>  drivers/staging/media/Kconfig                     |   2 +
>  drivers/staging/media/Makefile                    |   1 +
>  drivers/staging/media/imx/Kconfig                 |   8 +
>  drivers/staging/media/imx/Makefile                |   6 +
>  drivers/staging/media/imx/TODO                    |  22 +
>  drivers/staging/media/imx/imx-media-common.c      | 981 ++++++++++++++++++++++
>  drivers/staging/media/imx/imx-media-dev.c         | 486 +++++++++++
>  drivers/staging/media/imx/imx-media-fim.c         | 471 +++++++++++
>  drivers/staging/media/imx/imx-media-internal-sd.c | 457 ++++++++++
>  drivers/staging/media/imx/imx-media-of.c          | 289 +++++++
>  drivers/staging/media/imx/imx-media.h             | 310 +++++++
>  include/media/imx.h                               |  15 +
>  include/uapi/linux/v4l2-controls.h                |   4 +
>  14 files changed, 3495 insertions(+)
>  create mode 100644 Documentation/media/v4l-drivers/imx.rst
>  create mode 100644 drivers/staging/media/imx/Kconfig
>  create mode 100644 drivers/staging/media/imx/Makefile
>  create mode 100644 drivers/staging/media/imx/TODO
>  create mode 100644 drivers/staging/media/imx/imx-media-common.c
>  create mode 100644 drivers/staging/media/imx/imx-media-dev.c
>  create mode 100644 drivers/staging/media/imx/imx-media-fim.c
>  create mode 100644 drivers/staging/media/imx/imx-media-internal-sd.c
>  create mode 100644 drivers/staging/media/imx/imx-media-of.c
>  create mode 100644 drivers/staging/media/imx/imx-media.h
>  create mode 100644 include/media/imx.h
> 
> diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
> new file mode 100644
> index 0000000..87b37b5
> --- /dev/null
> +++ b/Documentation/media/v4l-drivers/imx.rst
> @@ -0,0 +1,443 @@
> +i.MX Video Capture Driver
> +=========================
> +
> +Introduction
> +------------
> +
> +The Freescale i.MX5/6 contains an Image Processing Unit (IPU), which
> +handles the flow of image frames to and from capture devices and
> +display devices.
> +
> +For image capture, the IPU contains the following internal subunits:
> +
> +- Image DMA Controller (IDMAC)
> +- Camera Serial Interface (CSI)
> +- Image Converter (IC)
> +- Sensor Multi-FIFO Controller (SMFC)
> +- Image Rotator (IRT)
> +- Video De-Interlace Controller (VDIC)

Nitpick: Video De-Interlacing or Combining Block (VDIC)

> +
> +The IDMAC is the DMA controller for transfer of image frames to and from
> +memory. Various dedicated DMA channels exist for both video capture and
> +display paths.
> +
> +The CSI is the frontend capture unit that interfaces directly with
> +capture sensors over Parallel, BT.656/1120, and MIPI CSI-2 busses.
> +
> +The IC handles color-space conversion, resizing, and rotation
> +operations. 

And horizontal flipping.

> There are three independent "tasks" within the IC that can
> +carry out conversions concurrently: pre-processing encoding,
> +pre-processing preview, and post-processing.

s/preview/viewfinder/ seems to be the commonly used name.

This paragraph could mention that a single hardware unit is used
transparently time multiplexed by the three tasks at different
granularity for the downsizing, main processing, and rotation sections.
The downscale unit switches between tasks at 8-pixel burst granularity,
the main processing unit at line granularity. The rotation units switch
only at frame granularity.

> +The SMFC is composed of four independent channels that each can transfer
> +captured frames from sensors directly to memory concurrently.
> +
> +The IRT carries out 90 and 270 degree image rotation operations.

... on 8x8 pixel blocks, supported by the IDMAC which handles block
transfers, block reordering, and vertical flipping.

> +The VDIC handles the conversion of interlaced video to progressive, with
> +support for different motion compensation modes (low, medium, and high
> +motion). The deinterlaced output frames from the VDIC can be sent to the
> +IC pre-process preview task for further conversions.
> +
> +In addition to the IPU internal subunits, there are also two units
> +outside the IPU that are also involved in video capture on i.MX:
> +
> +- MIPI CSI-2 Receiver for camera sensors with the MIPI CSI-2 bus
> +  interface. This is a Synopsys DesignWare core.
> +- A video multiplexer for selecting among multiple sensor inputs to
> +  send to a CSI.

Two of them, actually.

> +For more info, refer to the latest versions of the i.MX5/6 reference
> +manuals listed under References.
> +
> +
> +Features
> +--------
> +
> +Some of the features of this driver include:
> +
> +- Many different pipelines can be configured via media controller API,
> +  that correspond to the hardware video capture pipelines supported in
> +  the i.MX.
> +
> +- Supports parallel, BT.565, and MIPI CSI-2 interfaces.
> +
> +- Up to four concurrent sensor acquisitions, by configuring each
> +  sensor's pipeline using independent entities. This is currently
> +  demonstrated with the SabreSD and SabreLite reference boards with
> +  independent OV5642 and MIPI CSI-2 OV5640 sensor modules.
> +
> +- Scaling, color-space conversion, and image rotation via IC task
> +  subdevs.
> +
> +- Many pixel formats supported (RGB, packed and planar YUV, partial
> +  planar YUV).
> +
> +- The IC pre-process preview subdev supports motion compensated
> +  de-interlacing using the VDIC, with three motion compensation modes:
> +  low, medium, and high motion. The mode is specified with a custom
> +  control. Pipelines are defined that allow sending frames to the
> +  preview subdev directly from the CSI or from the SMFC.
> +
> +- Includes a Frame Interval Monitor (FIM) that can correct vertical sync
> +  problems with the ADV718x video decoders. See below for a description
> +  of the FIM.

Could this also be used to calculate more precise capture timestamps?

> +Capture Pipelines
> +-----------------
> +
> +The following describe the various use-cases supported by the pipelines.
> +
> +The links shown do not include the frontend sensor, video mux, or mipi
> +csi-2 receiver links. This depends on the type of sensor interface
> +(parallel or mipi csi-2). So in all cases, these pipelines begin with:
> +
> +sensor -> ipu_csi_mux -> ipu_csi -> ...
> +
> +for parallel sensors, or:
> +
> +sensor -> imx-mipi-csi2 -> (ipu_csi_mux) -> ipu_csi -> ...
> +
> +for mipi csi-2 sensors. The imx-mipi-csi2 receiver may need to route
> +to the video mux (ipu_csi_mux) before sending to the CSI, depending
> +on the mipi csi-2 virtual channel, hence ipu_csi_mux is shown in
> +parenthesis.
> +
> +Unprocessed Video Capture:
> +--------------------------
> +
> +Send frames directly from sensor to camera interface, with no
> +conversions:
> +
> +-> ipu_smfc -> camif

I'd call this capture interface, this is not just for cameras. Or maybe
idmac if you want to mirror hardware names?

> +Note the ipu_smfc can do pixel reordering within the same colorspace.

That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).

> +For example, its sink pad can take UYVY2X8, but its source pad can
> +output YUYV2X8.

I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
in the CSI chapter, for 8-bit per component data, the internal format
between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
(or "bayer/generic data"). In either case, the internal format does not
change along the way.

> +IC Direct Conversions:
> +----------------------
> +
> +This pipeline uses the preprocess encode entity to route frames directly
> +from the CSI to the IC (bypassing the SMFC), to carry out scaling up to
> +1024x1024 resolution, CSC, and image rotation:
> +
> +-> ipu_ic_prpenc -> camif
> +
> +This can be a useful capture pipeline for heavily loaded memory bus
> +traffic environments, since it has minimal IDMAC channel usage.

Note that if rotation is enabled, transfers between IC processing and
rotation still have to go through memory once.

> +Post-Processing Conversions:
> +----------------------------
> +
> +This pipeline routes frames from the SMFC to the post-processing
> +entity.

No, frames written by the CSI -> SMFC -> IDMAC path are read back into
the post-processing entity.

>  In addition to CSC and rotation, this entity supports tiling
> +which allows scaled output beyond the 1024x1024 limitation of the IC
> +(up to 4096x4096 scaling output is supported):
> +
> +-> ipu_smfc -> ipu_ic_pp -> camif
> +
> +Motion Compensated De-interlace:
> +--------------------------------
> +
> +This pipeline routes frames from the SMFC to the preprocess preview
> +entity to support motion-compensated de-interlacing using the VDIC,
> +scaling up to 1024x1024, and CSC:
> +
> +-> ipu_smfc -> ipu_ic_prpvf -> camif

Same as above.

> +This pipeline also carries out the same conversions as above, but routes
> +frames directly from the CSI to the IC preprocess preview entity for
> +minimal memory bandwidth usage (note: this pipeline only works in
> +"high motion" mode):
> +
> +-> ipu_ic_prpvf -> camif
> +
> +This pipeline takes the motion-compensated de-interlaced frames and
> +sends them to the post-processor, to support motion-compensated
> +de-interlacing, scaling up to 4096x4096, CSC, and rotation:
> +
> +-> (ipu_smfc) -> ipu_ic_prpvf -> ipu_ic_pp -> camif
> +
> +
> +Usage Notes
> +-----------
[...]
> +SabreLite with OV5642 and OV5640
> +--------------------------------
> +
> +This platform requires the OmniVision OV5642 module with a parallel
> +camera interface, and the OV5640 module with a MIPI CSI-2
> +interface. Both modules are available from Boundary Devices:
> +
> +https://boundarydevices.com/products/nit6x_5mp
> +https://boundarydevices.com/product/nit6x_5mp_mipi
> +
> +Note that if only one camera module is available, the other sensor
> +node can be disabled in the device tree.
> +
> +The OV5642 module is connected to the parallel bus input on the i.MX
> +internal video mux to IPU1 CSI0. It's i2c bus connects to i2c bus 2.
> +
> +The MIPI CSI-2 OV5640 module is connected to the i.MX internal MIPI CSI-2
> +receiver, and the four virtual channel outputs from the receiver are
> +routed as follows: vc0 to the IPU1 CSI0 mux, vc1 directly to IPU1 CSI1,
> +vc2 directly to IPU2 CSI0, and vc3 to the IPU2 CSI1 mux. The OV5640 is
> +also connected to i2c bus 2 on the SabreLite, therefore the OV5642 and
> +OV5640 must not share the same i2c slave address.
> +
> +The following basic example configures unprocessed video capture
> +pipelines for both sensors. The OV5642 is routed to camif0
> +(usually /dev/video0), and the OV5640 (transmitting on mipi csi-2
> +virtual channel 1) is routed to camif1 (usually /dev/video1). Both
> +sensors are configured to output 640x480, UYVY (not shown: all pad
> +field types should be set to "NONE"):
> +
> +.. code-block:: none
> +
> +   # Setup links for OV5642
> +   media-ctl -l '"ov5642 1-0042":0 -> "ipu1_csi0_mux":1[1]'
> +   media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> +   media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
> +   media-ctl -l '"ipu1_smfc0":1 -> "camif0":0[1]'
> +   media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
> +   # Setup links for OV5640
> +   media-ctl -l '"ov5640_mipi 1-0040":0 -> "imx-mipi-csi2":0[1]'
> +   media-ctl -l '"imx-mipi-csi2":2 -> "ipu1_csi1":0[1]'
> +   media-ctl -l '"ipu1_csi1":1 -> "ipu1_smfc1":0[1]'
> +   media-ctl -l '"ipu1_smfc1":1 -> "camif1":0[1]'
> +   media-ctl -l '"camif1":1 -> "camif1 devnode":0[1]'
> +   # Configure pads for OV5642 pipeline
> +   media-ctl -V "\"ov5642 1-0042\":0 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi0\":0 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi0\":1 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_smfc0\":0 [fmt:YUYV2X8/640x480]"
> +   media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/640x480]"

I think the smfc entities should be dropped.

> +   media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
> +   # Configure pads for OV5640 pipeline
> +   media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
[...]
> +   media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"

I agree this looks very intuitive, but technically correct for the
csi1:1 and camif1:0 pads would be a 32-bit YUV format.
(MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).

I think it would be better to use the correct format as that will allow
to chose the regular vs. companded packings in the future for formats
with more than 8 bits per component.

> +   media-ctl -V "\"camif1\":1 [fmt:UYVY2X8/640x480]"
>+
> +Streaming can then begin independently on device nodes /dev/video0
> +and /dev/video1.
> +
> +SabreAuto with ADV7180 decoder
> +------------------------------
> +
> +On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
> +parallel bus input on the internal video mux to IPU1 CSI0.
> +
> +The following example configures a pipeline to capture from the ADV7180
> +video decoder, assuming NTSC 720x480 input signals, with Motion
> +Compensated de-interlacing (not shown: all pad field types should be set
> +as indicated). $outputfmt can be any format supported by the
> +ipu1_ic_prpvf entity at its output pad:
> +
> +.. code-block:: none
> +
> +   # Setup links
> +   media-ctl -l '"adv7180 3-0021":0 -> "ipu1_csi0_mux":1[1]'
> +   media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> +   media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
> +   media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
> +   media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
> +   media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
> +   # Configure pads
> +   # pad field types for below pads must be an interlaced type
> +   # such as "ALTERNATE"

I think alternate should only extend as far as the CSI, since the CSI
can only capture NTSC/PAL fields in a fixed order.

> +   media-ctl -V "\"adv7180 3-0021\":0 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"

From here the interlaced field type should be sequential in the correct
order depending on NTSC/PAL.

> +   media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_csi0\":0 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_csi0\":1 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_smfc0\":0 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "\"ipu1_ic_prpvf\":0 [fmt:UYVY2X8/720x480]"
> +   # pad field types for below pads must be "NONE"
> +   media-ctl -V "\"ipu1_ic_prpvf\":1 [fmt:$outputfmt]"
> +   media-ctl -V "\"camif0\":0 [fmt:$outputfmt]"
> +   media-ctl -V "\"camif0\":1 [fmt:$outputfmt]"
> +
> +Streaming can then begin on /dev/video0.
> +
> +This platform accepts Composite Video analog inputs to the ADV7180 on
> +Ain1 (connector J42) and Ain3 (connector J43).
> +
> +To switch to Ain1:
> +
> +.. code-block:: none
> +
> +   # v4l2-ctl -i0
> +
> +To switch to Ain3:
> +
> +.. code-block:: none
> +
> +   # v4l2-ctl -i1
> +
> +
> +Frame Interval Monitor
> +----------------------
> +
> +The adv718x decoders can occasionally send corrupt fields during
> +NTSC/PAL signal re-sync (too little or too many video lines). When
> +this happens, the IPU triggers a mechanism to re-establish vertical
> +sync by adding 1 dummy line every frame, which causes a rolling effect
> +from image to image, and can last a long time before a stable image is
> +recovered. Or sometimes the mechanism doesn't work at all, causing a
> +permanent split image (one frame contains lines from two consecutive
> +captured images).

Is it only SabreAuto on which the FIM mechanism can be used due to the
pad routing?

[...]
> +/*
> + * DMA buffer ring handling
> + */
> +struct imx_media_dma_buf_ring {
> +	struct imx_media_dev *imxmd;
> +
> +	/* the ring */
> +	struct imx_media_dma_buf buf[IMX_MEDIA_MAX_RING_BUFS];
> +	/* the scratch buffer for underruns */
> +	struct imx_media_dma_buf scratch;
> +
> +	/* buffer generator */
> +	struct media_entity *src;
> +	/* buffer receiver */
> +	struct media_entity *sink;
> +
> +	spinlock_t lock;
> +
> +	int num_bufs;
> +	unsigned long last_seq;
> +};

I don't think this belongs in the capture driver at all.
Memory-to-memory transfers should be handled at the videobuf2 level.

[...]
> +static struct imx_media_dma_buf *
> +__dma_buf_queue(struct imx_media_dma_buf_ring *ring, int index)
> +{
> +	struct imx_media_dma_buf *buf;
> +
> +	if (index >= ring->num_bufs)
> +		return ERR_PTR(-EINVAL);
> +
> +	buf = &ring->buf[index];
> +	if (WARN_ON(buf->state != IMX_MEDIA_BUF_STATUS_PREPARED))
> +		return ERR_PTR(-EINVAL);
> +
> +	buf->state = IMX_MEDIA_BUF_STATUS_QUEUED;
> +	buf->seq = ring->last_seq++;
> +
> +	return buf;
> +}

Is this a whole software buffer queue implementation? I thought the
whole point of putting the custom mem2mem framework into the capture
driver was to use the hardware FSU channel linking?

> +int imx_media_dma_buf_queue(struct imx_media_dma_buf_ring *ring, int index)
> +{
> +	struct imx_media_dma_buf *buf;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ring->lock, flags);
> +	buf = __dma_buf_queue(ring, index);
> +	spin_unlock_irqrestore(&ring->lock, flags);
> +
> +	if (IS_ERR(buf))
> +		return PTR_ERR(buf);
> +
> +	dev_dbg(ring->imxmd->dev, "buf%d [%s -> %s] queued\n",
> +		index, ring->src->name, ring->sink->name);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_dma_buf_queue);
> +
> +int imx_media_dma_buf_queue_from_vb(struct imx_media_dma_buf_ring *ring,
> +				    struct vb2_buffer *vb)
> +{
> +	struct imx_media_dma_buf *buf;
> +	unsigned long flags;
> +	dma_addr_t phys;
> +	void *virt;
> +
> +	if (vb->index >= ring->num_bufs)
> +		return -EINVAL;
> +
> +	virt = vb2_plane_vaddr(vb, 0);
> +	phys = vb2_dma_contig_plane_dma_addr(vb, 0);
> +
> +	spin_lock_irqsave(&ring->lock, flags);
> +	buf = __dma_buf_queue(ring, vb->index);
> +	if (IS_ERR(buf))
> +		goto err_unlock;
> +
> +	buf->virt = virt;
> +	buf->phys = phys;
> +	buf->vb = vb;
> +	spin_unlock_irqrestore(&ring->lock, flags);
> +
> +	dev_dbg(ring->imxmd->dev, "buf%d [%s -> %s] queued from vb\n",
> +		buf->index, ring->src->name, ring->sink->name);
> +
> +	return 0;
> +err_unlock:
> +	spin_unlock_irqrestore(&ring->lock, flags);
> +	return PTR_ERR(buf);
> +}
> +EXPORT_SYMBOL_GPL(imx_media_dma_buf_queue_from_vb);
> +
> +void imx_media_dma_buf_done(struct imx_media_dma_buf *buf,
> +			    enum imx_media_dma_buf_status status)
> +{
> +	struct imx_media_dma_buf_ring *ring = buf->ring;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ring->lock, flags);
> +	WARN_ON(buf->state != IMX_MEDIA_BUF_STATUS_ACTIVE);
> +	buf->state = buf->status = status;
> +	spin_unlock_irqrestore(&ring->lock, flags);
> +
> +	if (buf == &ring->scratch)
> +		dev_dbg(ring->imxmd->dev, "buf-scratch [%s -> %s] done\n",
> +			ring->src->name, ring->sink->name);
> +	else
> +		dev_dbg(ring->imxmd->dev, "buf%d [%s -> %s] done\n",
> +			buf->index, ring->src->name, ring->sink->name);
> +
> +	/* if the sink is a subdev, inform it that new buffers are available */
> +	if (is_media_entity_v4l2_subdev(ring->sink)) {
> +		struct v4l2_subdev *sd =
> +			media_entity_to_v4l2_subdev(ring->sink);
> +		v4l2_subdev_call(sd, core, ioctl, IMX_MEDIA_NEW_DMA_BUF, NULL);

What is the purpose of this if the sink should be triggered by the FSU?

[...]
> +/*
> + * The subdevs have to be powered on/off, and streaming
> + * enabled/disabled, in a specific sequence.
> + */
> +static const u32 stream_on_seq[] = {
> +	IMX_MEDIA_GRP_ID_IC_PP,
> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> +	IMX_MEDIA_GRP_ID_SMFC,
> +	IMX_MEDIA_GRP_ID_SENSOR,
> +	IMX_MEDIA_GRP_ID_CSI2,
> +	IMX_MEDIA_GRP_ID_VIDMUX,
> +	IMX_MEDIA_GRP_ID_CSI,
> +};
> +
> +static const u32 stream_off_seq[] = {
> +	IMX_MEDIA_GRP_ID_IC_PP,
> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> +	IMX_MEDIA_GRP_ID_SMFC,
> +	IMX_MEDIA_GRP_ID_CSI,
> +	IMX_MEDIA_GRP_ID_VIDMUX,
> +	IMX_MEDIA_GRP_ID_CSI2,
> +	IMX_MEDIA_GRP_ID_SENSOR,
> +};
> +
> +#define NUM_STREAM_ENTITIES ARRAY_SIZE(stream_on_seq)
> +
> +static const u32 power_on_seq[] = {
> +	IMX_MEDIA_GRP_ID_CSI2,
> +	IMX_MEDIA_GRP_ID_SENSOR,
> +	IMX_MEDIA_GRP_ID_VIDMUX,
> +	IMX_MEDIA_GRP_ID_CSI,
> +	IMX_MEDIA_GRP_ID_SMFC,
> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> +	IMX_MEDIA_GRP_ID_IC_PP,
> +};
> +
> +static const u32 power_off_seq[] = {
> +	IMX_MEDIA_GRP_ID_IC_PP,
> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> +	IMX_MEDIA_GRP_ID_SMFC,
> +	IMX_MEDIA_GRP_ID_CSI,
> +	IMX_MEDIA_GRP_ID_VIDMUX,
> +	IMX_MEDIA_GRP_ID_SENSOR,
> +	IMX_MEDIA_GRP_ID_CSI2,
> +};

This seems somewhat arbitrary. Why is a power sequence needed?

[...]
> +/*
> + * Turn current pipeline power on/off starting from start_entity.
> + * Must be called with mdev->graph_mutex held.
> + */
> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
> +				 struct media_entity_graph *graph,
> +				 struct media_entity *start_entity, bool on)
> +{
> +	struct media_entity *entity;
> +	struct v4l2_subdev *sd;
> +	int i, ret = 0;
> +	u32 id;
> +
> +	for (i = 0; i < NUM_POWER_ENTITIES; i++) {
> +		id = on ? power_on_seq[i] : power_off_seq[i];
> +		entity = find_pipeline_entity(imxmd, graph, start_entity, id);
> +		if (!entity)
> +			continue;
> +
> +		sd = media_entity_to_v4l2_subdev(entity);
> +
> +		ret = v4l2_subdev_call(sd, core, s_power, on);
> +		if (ret && ret != -ENOIOCTLCMD)
> +			break;
> +	}
> +
> +	return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);

This should really be handled by v4l2_pipeline_pm_use.

> +/*
> + * Inherit the v4l2 controls from all entities in a pipeline
> + * to the given video device.
> + * Must be called with mdev->graph_mutex held.
> + */
> +int imx_media_inherit_controls(struct imx_media_dev *imxmd,
> +			       struct video_device *vfd,
> +			       struct media_entity *start_entity)
> +{
> +	struct media_entity_graph graph;
> +	struct media_entity *entity;
> +	struct v4l2_subdev *sd;
> +	int ret;
> +
> +	ret = media_entity_graph_walk_init(&graph, &imxmd->md);
> +	if (ret)
> +		return ret;
> +
> +	media_entity_graph_walk_start(&graph, start_entity);
> +
> +	while ((entity = media_entity_graph_walk_next(&graph))) {
> +		if (is_media_entity_v4l2_video_device(entity))
> +			continue;
> +
> +		sd = media_entity_to_v4l2_subdev(entity);
> +
> +		dev_dbg(imxmd->dev, "%s: adding controls from %s\n",
> +			__func__, sd->name);
> +
> +		ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
> +					    sd->ctrl_handler,
> +					    NULL);
> +		if (ret)
> +			break;
> +	}
> +
> +	media_entity_graph_walk_cleanup(&graph);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(imx_media_inherit_controls);
> +
> +MODULE_DESCRIPTION("i.MX5/6 v4l2 media controller driver");
> +MODULE_AUTHOR("Steve Longerbeam <steve_longerbeam@mentor.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
> new file mode 100644
> index 0000000..357654d
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-dev.c

This file is full of code that should live in the v4l2 core.

[...]
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-internal-sd.c
[...]
> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
> +				   struct imx_media_subdev *csi[4])
> +{
> +	int ret;
> +
> +	/* there must be at least one CSI in first IPU */

Why?

> +	if (!(csi[0] || csi[1]))
> +		return -EINVAL;
> +
> +	ret = add_ipu_internal_subdevs(imxmd, csi[0], csi[1], 0);
> +	if (ret)
> +		return ret;
> +
> +	if (csi[2] || csi[3])
> +		ret = add_ipu_internal_subdevs(imxmd, csi[2], csi[3], 1);
> +
> +	return ret;
> +}
> diff --git a/drivers/staging/media/imx/imx-media-of.c b/drivers/staging/media/imx/imx-media-of.c
> new file mode 100644
> index 0000000..a939c34
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx-media-of.c
> @@ -0,0 +1,289 @@
> +/*
> + * Media driver for Freescale i.MX5/6 SOC
> + *
> + * Open Firmware parsing.
> + *
> + * Copyright (c) 2016 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.
> + */
> +#include <linux/of_platform.h>
> +#include <media/v4l2-ctrls.h>
> +#include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> +#include <media/v4l2-subdev.h>
> +#include <media/videobuf2-dma-contig.h>
> +#include <video/imx-ipu-v3.h>
> +#include "imx-media.h"
> +
> +static int of_add_pad_link(struct imx_media_dev *imxmd,
> +			   struct imx_media_pad *pad,
> +			   struct device_node *local_sd_node,
> +			   struct device_node *remote_sd_node,
> +			   int local_pad, int remote_pad)
> +{
> +	dev_dbg(imxmd->dev, "%s: adding %s:%d -> %s:%d\n", __func__,
> +		local_sd_node->name, local_pad,
> +		remote_sd_node->name, remote_pad);
> +
> +	return imx_media_add_pad_link(imxmd, pad, remote_sd_node, NULL,
> +				      local_pad, remote_pad);
> +}
> +
> +/* parse inputs property from a sensor node */
> +static void of_parse_sensor_inputs(struct imx_media_dev *imxmd,
> +				   struct imx_media_subdev *sensor,
> +				   struct device_node *sensor_np)
> +{
> +	struct imx_media_sensor_input *sinput = &sensor->input;
> +	int ret, i;
> +
> +	for (i = 0; i < IMX_MEDIA_MAX_SENSOR_INPUTS; i++) {
> +		const char *input_name;
> +		u32 val;
> +
> +		ret = of_property_read_u32_index(sensor_np, "inputs", i, &val);
> +		if (ret)
> +			break;
> +
> +		sinput->value[i] = val;
> +
> +		ret = of_property_read_string_index(sensor_np, "input-names",
> +						    i, &input_name);
> +		/*
> +		 * if input-names not provided, they will be set using
> +		 * the subdev name once the sensor is known during
> +		 * async bind
> +		 */
> +		if (!ret)
> +			strncpy(sinput->name[i], input_name,
> +				sizeof(sinput->name[i]));
> +	}
> +
> +	sinput->num = i;
> +
> +	/* if no inputs provided just assume a single input */
> +	if (sinput->num == 0)
> +		sinput->num = 1;
> +}

This should be parsed by the sensor driver, not imx-media.

> +static void of_parse_sensor(struct imx_media_dev *imxmd,
> +			    struct imx_media_subdev *sensor,
> +			    struct device_node *sensor_np)
> +{
> +	struct device_node *endpoint;
> +
> +	of_parse_sensor_inputs(imxmd, sensor, sensor_np);
> +
> +	endpoint = of_graph_get_next_endpoint(sensor_np, NULL);
> +	if (endpoint) {
> +		v4l2_of_parse_endpoint(endpoint, &sensor->sensor_ep);
> +		of_node_put(endpoint);
> +	}
> +}
> +
> +static int of_get_port_count(const struct device_node *np)
> +{
> +	struct device_node *child;
> +	int num = 0;
> +
> +	/* if this node is itself a port, return 1 */
> +	if (of_node_cmp(np->name, "port") == 0)
> +		return 1;
> +
> +	for_each_child_of_node(np, child)
> +		if (of_node_cmp(child->name, "port") == 0)
> +			num++;
> +
> +	return num;
> +}

If this is extended to handle the ports subnode properly, it could be
moved into drivers/of/base.c.

regards
Philipp

[toc] | [prev] | [next] | [standalone]


#1559087 — Re: [PATCH v3 16/24] media: Add i.MX media core driver

FromSteve Longerbeam <slongerbeam@gmail.com>
Date2017-01-14 23:50 +0100
SubjectRe: [PATCH v3 16/24] media: Add i.MX media core driver
Message-ID<sZFj3-3c7-1@gated-at.bofh.it>
In reply to#1558491
(sorry sending again w/o html)


On 01/13/2017 07:20 AM, Philipp Zabel wrote:
> Am Freitag, den 06.01.2017, 18:11 -0800 schrieb Steve Longerbeam:
>> +For image capture, the IPU contains the following internal subunits:
>> +
>> +- Image DMA Controller (IDMAC)
>> +- Camera Serial Interface (CSI)
>> +- Image Converter (IC)
>> +- Sensor Multi-FIFO Controller (SMFC)
>> +- Image Rotator (IRT)
>> +- Video De-Interlace Controller (VDIC)
> Nitpick: Video De-Interlacing or Combining Block (VDIC)

done.

>> +
>> +The IDMAC is the DMA controller for transfer of image frames to and from
>> +memory. Various dedicated DMA channels exist for both video capture and
>> +display paths.
>> +
>> +The CSI is the frontend capture unit that interfaces directly with
>> +capture sensors over Parallel, BT.656/1120, and MIPI CSI-2 busses.
>> +
>> +The IC handles color-space conversion, resizing, and rotation
>> +operations.
> And horizontal flipping.

done.


>> There are three independent "tasks" within the IC that can
>> +carry out conversions concurrently: pre-processing encoding,
>> +pre-processing preview, and post-processing.
> s/preview/viewfinder/ seems to be the commonly used name.

replaced everywhere in the doc.

> This paragraph could mention that a single hardware unit is used
> transparently time multiplexed by the three tasks at different
> granularity for the downsizing, main processing, and rotation sections.
> The downscale unit switches between tasks at 8-pixel burst granularity,
> the main processing unit at line granularity. The rotation units switch
> only at frame granularity.

I've added that info.

>> +The SMFC is composed of four independent channels that each can transfer
>> +captured frames from sensors directly to memory concurrently.
>> +
>> +The IRT carries out 90 and 270 degree image rotation operations.
> ... on 8x8 pixel blocks, supported by the IDMAC which handles block
> transfers, block reordering, and vertical flipping.

done.

>> +The VDIC handles the conversion of interlaced video to progressive, with
>> +support for different motion compensation modes (low, medium, and high
>> +motion). The deinterlaced output frames from the VDIC can be sent to the
>> +IC pre-process preview task for further conversions.
>> +
>> +In addition to the IPU internal subunits, there are also two units
>> +outside the IPU that are also involved in video capture on i.MX:
>> +
>> +- MIPI CSI-2 Receiver for camera sensors with the MIPI CSI-2 bus
>> +  interface. This is a Synopsys DesignWare core.
>> +- A video multiplexer for selecting among multiple sensor inputs to
>> +  send to a CSI.
> Two of them, actually.

done.

>> +
>> +- Includes a Frame Interval Monitor (FIM) that can correct vertical sync
>> +  problems with the ADV718x video decoders. See below for a description
>> +  of the FIM.
> Could this also be used to calculate more precise capture timestamps?

An input capture function could do that, triggered off a VSYNC or FIELD
signal such as on the ADV718x. The FIM is only used to calculate
frame intervals at this point, but its input capture method could be
used to also record more accurate timestamps.


>> +Capture Pipelines
>> +-----------------
>> +
>> +The following describe the various use-cases supported by the pipelines.
>> +
>> +The links shown do not include the frontend sensor, video mux, or mipi
>> +csi-2 receiver links. This depends on the type of sensor interface
>> +(parallel or mipi csi-2). So in all cases, these pipelines begin with:
>> +
>> +sensor -> ipu_csi_mux -> ipu_csi -> ...
>> +
>> +for parallel sensors, or:
>> +
>> +sensor -> imx-mipi-csi2 -> (ipu_csi_mux) -> ipu_csi -> ...
>> +
>> +for mipi csi-2 sensors. The imx-mipi-csi2 receiver may need to route
>> +to the video mux (ipu_csi_mux) before sending to the CSI, depending
>> +on the mipi csi-2 virtual channel, hence ipu_csi_mux is shown in
>> +parenthesis.
>> +
>> +Unprocessed Video Capture:
>> +--------------------------
>> +
>> +Send frames directly from sensor to camera interface, with no
>> +conversions:
>> +
>> +-> ipu_smfc -> camif
> I'd call this capture interface, this is not just for cameras. Or maybe
> idmac if you want to mirror hardware names?

Camif is so named because it is the V4L2 user interface for video
capture. I suppose it could be named "capif", but that doesn't role
off the tongue quite as well.

>> +Note the ipu_smfc can do pixel reordering within the same colorspace.
> That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).

yes, the doc is re-worded to make that more clear.

>> +For example, its sink pad can take UYVY2X8, but its source pad can
>> +output YUYV2X8.
> I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
> in the CSI chapter, for 8-bit per component data, the internal format
> between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
> (or "bayer/generic data"). In either case, the internal format does not
> change along the way.

these are pixels in memory buffers, not the IPU internal formats.


>> +IC Direct Conversions:
>> +----------------------
>> +
>> +This pipeline uses the preprocess encode entity to route frames directly
>> +from the CSI to the IC (bypassing the SMFC), to carry out scaling up to
>> +1024x1024 resolution, CSC, and image rotation:
>> +
>> +-> ipu_ic_prpenc -> camif
>> +
>> +This can be a useful capture pipeline for heavily loaded memory bus
>> +traffic environments, since it has minimal IDMAC channel usage.
> Note that if rotation is enabled, transfers between IC processing and
> rotation still have to go through memory once.

yep.

>> +Post-Processing Conversions:
>> +----------------------------
>> +
>> +This pipeline routes frames from the SMFC to the post-processing
>> +entity.
> No, frames written by the CSI -> SMFC -> IDMAC path are read back into
> the post-processing entity.

that's true. The post-processing entity kicks off its read channels
to transfer those frames into the post-processor. Anyway this wording
will change after doing away with the SMFC entity.


>> +   media-ctl -V "\"ipu1_csi0\":1 [fmt:YUYV2X8/640x480]"
>> +   media-ctl -V "\"ipu1_smfc0\":0 [fmt:YUYV2X8/640x480]"
>> +   media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/640x480]"
> I think the smfc entities should be dropped.

yes working on that.

>> +   media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
>> +   media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
>> +   # Configure pads for OV5640 pipeline
>> +   media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
>> +   media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
>> +   media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
>> +   media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
>> +   media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
> [...]
>> +   media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
> I agree this looks very intuitive, but technically correct for the
> csi1:1 and camif1:0 pads would be a 32-bit YUV format.
> (MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
>
> I think it would be better to use the correct format

I'm not sure I follow you here.

>   as that will allow
> to chose the regular vs. companded packings in the future for formats
> with more than 8 bits per component.
>
>> +   media-ctl -V "\"camif1\":1 [fmt:UYVY2X8/640x480]"
>> +
>> +Streaming can then begin independently on device nodes /dev/video0
>> +and /dev/video1.
>> +
>> +SabreAuto with ADV7180 decoder
>> +------------------------------
>> +
>> +On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>> +parallel bus input on the internal video mux to IPU1 CSI0.
>> +
>> +The following example configures a pipeline to capture from the ADV7180
>> +video decoder, assuming NTSC 720x480 input signals, with Motion
>> +Compensated de-interlacing (not shown: all pad field types should be set
>> +as indicated). $outputfmt can be any format supported by the
>> +ipu1_ic_prpvf entity at its output pad:
>> +
>> +.. code-block:: none
>> +
>> +   # Setup links
>> +   media-ctl -l '"adv7180 3-0021":0 -> "ipu1_csi0_mux":1[1]'
>> +   media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>> +   media-ctl -l '"ipu1_csi0":1 -> "ipu1_smfc0":0[1]'
>> +   media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
>> +   media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
>> +   media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
>> +   # Configure pads
>> +   # pad field types for below pads must be an interlaced type
>> +   # such as "ALTERNATE"
> I think alternate should only extend as far as the CSI, since the CSI
> can only capture NTSC/PAL fields in a fixed order.

Agreed, I'm doing the translation from alternate to seq_bt/seq_tb depending
on sensor std in imx-vdic.c, but it should move upstream. Will fix.

>> +   media-ctl -V "\"adv7180 3-0021\":0 [fmt:UYVY2X8/720x480]"
>> +   media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
>  From here the interlaced field type should be sequential in the correct
> order depending on NTSC/PAL.

right.

>> +
>> +Frame Interval Monitor
>> +----------------------
>> +
>> +The adv718x decoders can occasionally send corrupt fields during
>> +NTSC/PAL signal re-sync (too little or too many video lines). When
>> +this happens, the IPU triggers a mechanism to re-establish vertical
>> +sync by adding 1 dummy line every frame, which causes a rolling effect
>> +from image to image, and can last a long time before a stable image is
>> +recovered. Or sometimes the mechanism doesn't work at all, causing a
>> +permanent split image (one frame contains lines from two consecutive
>> +captured images).
> Is it only SabreAuto on which the FIM mechanism can be used due to the
> pad routing?

No, FIM can be used on any target, the fim child node of ipu_csi just
needs to be enabled via status property.

> [...]
>> +/*
>> + * DMA buffer ring handling
>> + */
>> +struct imx_media_dma_buf_ring {
>> +	struct imx_media_dev *imxmd;
>> +
>> +	/* the ring */
>> +	struct imx_media_dma_buf buf[IMX_MEDIA_MAX_RING_BUFS];
>> +	/* the scratch buffer for underruns */
>> +	struct imx_media_dma_buf scratch;
>> +
>> +	/* buffer generator */
>> +	struct media_entity *src;
>> +	/* buffer receiver */
>> +	struct media_entity *sink;
>> +
>> +	spinlock_t lock;
>> +
>> +	int num_bufs;
>> +	unsigned long last_seq;
>> +};
> I don't think this belongs in the capture driver at all.
> Memory-to-memory transfers should be handled at the videobuf2 level.

see below.

> [...]
>> +static struct imx_media_dma_buf *
>> +__dma_buf_queue(struct imx_media_dma_buf_ring *ring, int index)
>> +{
>> +	struct imx_media_dma_buf *buf;
>> +
>> +	if (index >= ring->num_bufs)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	buf = &ring->buf[index];
>> +	if (WARN_ON(buf->state != IMX_MEDIA_BUF_STATUS_PREPARED))
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	buf->state = IMX_MEDIA_BUF_STATUS_QUEUED;
>> +	buf->seq = ring->last_seq++;
>> +
>> +	return buf;
>> +}
> Is this a whole software buffer queue implementation? I thought the
> whole point of putting the custom mem2mem framework into the capture
> driver was to use the hardware FSU channel linking?

  see below.

> What is the purpose of this if the sink should be triggered by the FSU?

Ok, here is where I need to make an admission.

The only FSU links I have attempted (and which currently have entries
in the fsu_link_info[] table), are the enc/vf/pp --> IRT links for rotation.

There does not appear to be support in the FSU for linking a write channel
to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
is support for the direct link from CSI (which I am using), but that's 
not an
IDMAC channel link.

There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
0..2 (and 3? it's not clear, and not clear whether this includes channel 1).
But I think this links to channel 12, and not to channels 8,9,10 to the 
VDIC.
Or will it? It's worth experimenting. It would have helped if FSL listed 
which
IDMAC channels these FSU links correspond to, instead of making us guess
at it.

In any event, the docs are not clear enough to implement a real FSU
link to the VDIC read channels, if it's even possible. And trying to get
programming help from FSL can be difficult, and no coding examples
for this link AFAIK.

So I ended resorted to linking to VDIC channels 8,9,10 with a software
approach, instead of attempting a hardware FSU link.

The EOF interrupt handler for the SMFC channels informs the VDIC
entity via a v4l2_subdev_ioctl() call that a buffer is available. The
VDIC then manually kicks off its read channels to bring that buffer
(and a previous buffer for F(n-1) field) into the VDIC.

There is a small amount of extra overhead going this route compared
to a FSU hardware link: there is the EOF irq latency (a few usec), and
the CPU overhead for the VDIC to manually start the read channels,
which is also a few usec at most (see prepare_vdi_in_buffers() in
imx-vdic.c). So in total at most ~10 usec of extra overhead (CPU
use plus irq latency) under normal system load.

Of course, in order to implement this software link, I had to implement
a straightforward FIFO dma buffer ring. The sink (VDIC) allocates the ring
at stream on, and the source requests a pointer to this ring in its own
stream on. Passing buffers from source to sink then follows a 
straightforward
FIFO queue/done/dequeue/queue model: sink queues buffers to src, src
grabs queued buffers and makes them active, src signals completed
buffers to sink, sink dequeues buffers in response, and sink queues
buffers back when it is finished with them.


> [...]
>> +/*
>> + * The subdevs have to be powered on/off, and streaming
>> + * enabled/disabled, in a specific sequence.
>> + */
>> +static const u32 stream_on_seq[] = {
>> +	IMX_MEDIA_GRP_ID_IC_PP,
>> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
>> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
>> +	IMX_MEDIA_GRP_ID_SMFC,
>> +	IMX_MEDIA_GRP_ID_SENSOR,
>> +	IMX_MEDIA_GRP_ID_CSI2,
>> +	IMX_MEDIA_GRP_ID_VIDMUX,
>> +	IMX_MEDIA_GRP_ID_CSI,
>> +};
>> +
>> +static const u32 stream_off_seq[] = {
>> +	IMX_MEDIA_GRP_ID_IC_PP,
>> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
>> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
>> +	IMX_MEDIA_GRP_ID_SMFC,
>> +	IMX_MEDIA_GRP_ID_CSI,
>> +	IMX_MEDIA_GRP_ID_VIDMUX,
>> +	IMX_MEDIA_GRP_ID_CSI2,
>> +	IMX_MEDIA_GRP_ID_SENSOR,
>> +};
>> +
>> +#define NUM_STREAM_ENTITIES ARRAY_SIZE(stream_on_seq)
>> +
>> +static const u32 power_on_seq[] = {
>> +	IMX_MEDIA_GRP_ID_CSI2,
>> +	IMX_MEDIA_GRP_ID_SENSOR,
>> +	IMX_MEDIA_GRP_ID_VIDMUX,
>> +	IMX_MEDIA_GRP_ID_CSI,
>> +	IMX_MEDIA_GRP_ID_SMFC,
>> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
>> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
>> +	IMX_MEDIA_GRP_ID_IC_PP,
>> +};
>> +
>> +static const u32 power_off_seq[] = {
>> +	IMX_MEDIA_GRP_ID_IC_PP,
>> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
>> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
>> +	IMX_MEDIA_GRP_ID_SMFC,
>> +	IMX_MEDIA_GRP_ID_CSI,
>> +	IMX_MEDIA_GRP_ID_VIDMUX,
>> +	IMX_MEDIA_GRP_ID_SENSOR,
>> +	IMX_MEDIA_GRP_ID_CSI2,
>> +};
> This seems somewhat arbitrary. Why is a power sequence needed?

The CSI-2 receiver must be powered up before the sensor, that's the
only requirement IIRC. The others have no s_power requirement. So I
can probably change this to power up in the frontend -> backend order
(IC_PP to sensor). And vice-versa for power off.


> [...]
>> +/*
>> + * Turn current pipeline power on/off starting from start_entity.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
>> +				 struct media_entity_graph *graph,
>> +				 struct media_entity *start_entity, bool on)
>> +{
>> +	struct media_entity *entity;
>> +	struct v4l2_subdev *sd;
>> +	int i, ret = 0;
>> +	u32 id;
>> +
>> +	for (i = 0; i < NUM_POWER_ENTITIES; i++) {
>> +		id = on ? power_on_seq[i] : power_off_seq[i];
>> +		entity = find_pipeline_entity(imxmd, graph, start_entity, id);
>> +		if (!entity)
>> +			continue;
>> +
>> +		sd = media_entity_to_v4l2_subdev(entity);
>> +
>> +		ret = v4l2_subdev_call(sd, core, s_power, on);
>> +		if (ret && ret != -ENOIOCTLCMD)
>> +			break;
>> +	}
>> +
>> +	return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> This should really be handled by v4l2_pipeline_pm_use.

I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
doing some other stuff that bothered me, at least that's what I remember.
I will revisit this.

>> +/*
>> + * Inherit the v4l2 controls from all entities in a pipeline
>> + * to the given video device.
>> + * Must be called with mdev->graph_mutex held.
>> + */
>> +int imx_media_inherit_controls(struct imx_media_dev *imxmd,
>> +			       struct video_device *vfd,
>> +			       struct media_entity *start_entity)
>> +{
>> +	struct media_entity_graph graph;
>> +	struct media_entity *entity;
>> +	struct v4l2_subdev *sd;
>> +	int ret;
>> +
>> +	ret = media_entity_graph_walk_init(&graph, &imxmd->md);
>> +	if (ret)
>> +		return ret;
>> +
>> +	media_entity_graph_walk_start(&graph, start_entity);
>> +
>> +	while ((entity = media_entity_graph_walk_next(&graph))) {
>> +		if (is_media_entity_v4l2_video_device(entity))
>> +			continue;
>> +
>> +		sd = media_entity_to_v4l2_subdev(entity);
>> +
>> +		dev_dbg(imxmd->dev, "%s: adding controls from %s\n",
>> +			__func__, sd->name);
>> +
>> +		ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
>> +					    sd->ctrl_handler,
>> +					    NULL);
>> +		if (ret)
>> +			break;
>> +	}
>> +
>> +	media_entity_graph_walk_cleanup(&graph);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(imx_media_inherit_controls);
>> +
>> +MODULE_DESCRIPTION("i.MX5/6 v4l2 media controller driver");
>> +MODULE_AUTHOR("Steve Longerbeam<steve_longerbeam@mentor.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c b/drivers/staging/media/imx/imx-media-dev.c
>> new file mode 100644
>> index 0000000..357654d
>> --- /dev/null
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
> This file is full of code that should live in the v4l2 core.

Agreed. Stuff like imx_media_inherit_controls() really should be widely
available.

>> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
>> +				   struct imx_media_subdev *csi[4])
>> +{
>> +	int ret;
>> +
>> +	/* there must be at least one CSI in first IPU */
> Why?

Well yeah, imx-media doesn't necessarily need a CSI if things
like the VDIC or post-processor are being used by an output
overlay pipeline, for example. I'll fix this.

>> +
>> +/* parse inputs property from a sensor node */
>> +static void of_parse_sensor_inputs(struct imx_media_dev *imxmd,
>> +				   struct imx_media_subdev *sensor,
>> +				   struct device_node *sensor_np)
>> +{
>> +	struct imx_media_sensor_input *sinput = &sensor->input;
>> +	int ret, i;
>> +
>> +	for (i = 0; i < IMX_MEDIA_MAX_SENSOR_INPUTS; i++) {
>> +		const char *input_name;
>> +		u32 val;
>> +
>> +		ret = of_property_read_u32_index(sensor_np, "inputs", i, &val);
>> +		if (ret)
>> +			break;
>> +
>> +		sinput->value[i] = val;
>> +
>> +		ret = of_property_read_string_index(sensor_np, "input-names",
>> +						    i, &input_name);
>> +		/*
>> +		 * if input-names not provided, they will be set using
>> +		 * the subdev name once the sensor is known during
>> +		 * async bind
>> +		 */
>> +		if (!ret)
>> +			strncpy(sinput->name[i], input_name,
>> +				sizeof(sinput->name[i]));
>> +	}
>> +
>> +	sinput->num = i;
>> +
>> +	/* if no inputs provided just assume a single input */
>> +	if (sinput->num == 0)
>> +		sinput->num = 1;
>> +}
> This should be parsed by the sensor driver, not imx-media.

you're probably right. I'll submit a patch for adv7180.c.


>> +static void of_parse_sensor(struct imx_media_dev *imxmd,
>> +			    struct imx_media_subdev *sensor,
>> +			    struct device_node *sensor_np)
>> +{
>> +	struct device_node *endpoint;
>> +
>> +	of_parse_sensor_inputs(imxmd, sensor, sensor_np);
>> +
>> +	endpoint = of_graph_get_next_endpoint(sensor_np, NULL);
>> +	if (endpoint) {
>> +		v4l2_of_parse_endpoint(endpoint, &sensor->sensor_ep);
>> +		of_node_put(endpoint);
>> +	}
>> +}
>> +
>> +static int of_get_port_count(const struct device_node *np)
>> +{
>> +	struct device_node *child;
>> +	int num = 0;
>> +
>> +	/* if this node is itself a port, return 1 */
>> +	if (of_node_cmp(np->name, "port") == 0)
>> +		return 1;
>> +
>> +	for_each_child_of_node(np, child)
>> +		if (of_node_cmp(child->name, "port") == 0)
>> +			num++;
>> +
>> +	return num;
>> +}
> If this is extended to handle the ports subnode properly, it could be
> moved into drivers/of/base.c.

More that eventually needs exporting, agreed.


Steve

[toc] | [prev] | [next] | [standalone]


#1559747 — Re: [PATCH v3 16/24] media: Add i.MX media core driver

FromPhilipp Zabel <p.zabel@pengutronix.de>
Date2017-01-16 14:50 +0100
SubjectRe: [PATCH v3 16/24] media: Add i.MX media core driver
Message-ID<t0fPA-14g-11@gated-at.bofh.it>
In reply to#1559087
On Sat, 2017-01-14 at 14:46 -0800, Steve Longerbeam wrote:
[...]
> >> +Unprocessed Video Capture:
> >> +--------------------------
> >> +
> >> +Send frames directly from sensor to camera interface, with no
> >> +conversions:
> >> +
> >> +-> ipu_smfc -> camif
> > I'd call this capture interface, this is not just for cameras. Or maybe
> > idmac if you want to mirror hardware names?
> 
> Camif is so named because it is the V4L2 user interface for video
> capture. I suppose it could be named "capif", but that doesn't role
> off the tongue quite as well.

Agreed, capif sounds weird. I find camif a bit confusing though, because
Samsung S3C has a camera interface that is actually called "CAMIF".

> >> +Note the ipu_smfc can do pixel reordering within the same colorspace.
> > That isn't a feature of the SMFC, but of the IDMAC (FCW & FCR).
> 
> yes, the doc is re-worded to make that more clear.
> 
> >> +For example, its sink pad can take UYVY2X8, but its source pad can
> >> +output YUYV2X8.
> > I don't think this is correct. Re-reading "37.4.3.7 Packing to memory"
> > in the CSI chapter, for 8-bit per component data, the internal format
> > between CSI, SMFC, and IDMAC is always some 32-bit RGBx/YUVx variant
> > (or "bayer/generic data"). In either case, the internal format does not
> > change along the way.
> 
> these are pixels in memory buffers, not the IPU internal formats.

As long as we are talking about the CSI -> SMFC -> IDMAC path, these
should be IPU internal formats. How else would one choose between 8-bit
companded RGB, and 16-bit expanded RGB for a 10-bit per component input
signal? This is the same issue as in the next comment.

> >> +   media-ctl -V "\"camif0\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"camif0\":1 [fmt:UYVY2X8/640x480]"
> >> +   # Configure pads for OV5640 pipeline
> >> +   media-ctl -V "\"ov5640_mipi 1-0040\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"imx-mipi-csi2\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"imx-mipi-csi2\":2 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"ipu1_csi1\":0 [fmt:UYVY2X8/640x480]"
> >> +   media-ctl -V "\"ipu1_csi1\":1 [fmt:UYVY2X8/640x480]"
> > [...]
> >> +   media-ctl -V "\"camif1\":0 [fmt:UYVY2X8/640x480]"
> > I agree this looks very intuitive, but technically correct for the
> > csi1:1 and camif1:0 pads would be a 32-bit YUV format.
> > (MEDIA_BUS_FMT_YUV8_1X32_PADLO doesn't exist yet).
> >
> > I think it would be better to use the correct format
> 
> I'm not sure I follow you here.

The ov5640 sends UYVY2X8 on the wire, so pads "ov5640_mipi 1-0040":0
up to "ipu1_csi1":0 are correct. But the CSI writes 32-bit YUV values
into the SMFC, so the CSI output pad and the IDMAC input pad should have
a YUV8_1X32 format.

Chapter 37.4.2.3 "FCW & FCR - Format converter write and read" in the
IDMAC chapter states that all internal submodules only work on 8-bit per
component formats with four components: YUVA or RGBA.

> > [...]
> > Is this a whole software buffer queue implementation? I thought the
> > whole point of putting the custom mem2mem framework into the capture
> > driver was to use the hardware FSU channel linking?
> 
>   see below.
> 
> > What is the purpose of this if the sink should be triggered by the FSU?
> 
> Ok, here is where I need to make an admission.
> 
> The only FSU links I have attempted (and which currently have entries
> in the fsu_link_info[] table), are the enc/vf/pp --> IRT links for rotation.

Which are not described as media entity links because the rotation units
do not have separate media entities. So me arguing against handling
mem2mem chaining via media entity links doesn't concern these implicit
links.

> There does not appear to be support in the FSU for linking a write channel
> to the VDIC read channels (8, 9, 10) according to VDI_SRC_SEL field. There
> is support for the direct link from CSI (which I am using), but that's 
> not an
> IDMAC channel link.
> 
> There is a PRP_SRC_SEL field, with linking from IDMAC (SMFC) channels
> 0..2 (and 3? it's not clear, and not clear whether this includes channel 1).

As I read it, that is 0 and 2 only, no idea why. But since there are
only 2 CSIs, that shouldn't be a problem.

> But I think this links to channel 12, and not to channels 8,9,10 to the 
> VDIC.
> Or will it? It's worth experimenting. It would have helped if FSL listed 
> which
> IDMAC channels these FSU links correspond to, instead of making us guess
> at it.

I would have assumed that the FSU triggering only works on 1:1 channels
and the VDIC with its three input channels is different. But then
there's the alleged VDOA link to ch8/9/10 ro ch9, depending on
VDI_MOT_SEL.

This makes me more convinced that the CSI -> VDIC link should only
describe the direct path (real-time mode, single field).

> In any event, the docs are not clear enough to implement a real FSU
> link to the VDIC read channels, if it's even possible. And trying to get
> programming help from FSL can be difficult, and no coding examples
> for this link AFAIK.
> 
> So I ended resorted to linking to VDIC channels 8,9,10 with a software
> approach, instead of attempting a hardware FSU link.
> 
> The EOF interrupt handler for the SMFC channels informs the VDIC
> entity via a v4l2_subdev_ioctl() call that a buffer is available. The
> VDIC then manually kicks off its read channels to bring that buffer
> (and a previous buffer for F(n-1) field) into the VDIC.
> 
> There is a small amount of extra overhead going this route compared
> to a FSU hardware link: there is the EOF irq latency (a few usec), and
> the CPU overhead for the VDIC to manually start the read channels,
> which is also a few usec at most (see prepare_vdi_in_buffers() in
> imx-vdic.c). So in total at most ~10 usec of extra overhead (CPU
> use plus irq latency) under normal system load.

That the same low overhead could be reached by linking videobuf2 queues
of different video devices, that would be a lot more flexible.

> Of course, in order to implement this software link, I had to implement
> a straightforward FIFO dma buffer ring. The sink (VDIC) allocates the ring
> at stream on, and the source requests a pointer to this ring in its own
> stream on. Passing buffers from source to sink then follows a 
> straightforward
> FIFO queue/done/dequeue/queue model: sink queues buffers to src, src
> grabs queued buffers and makes them active, src signals completed
> buffers to sink, sink dequeues buffers in response, and sink queues
> buffers back when it is finished with them.

Thank you for the explanation.

[...]
> >> +static const u32 power_off_seq[] = {
> >> +	IMX_MEDIA_GRP_ID_IC_PP,
> >> +	IMX_MEDIA_GRP_ID_IC_PRPVF,
> >> +	IMX_MEDIA_GRP_ID_IC_PRPENC,
> >> +	IMX_MEDIA_GRP_ID_SMFC,
> >> +	IMX_MEDIA_GRP_ID_CSI,
> >> +	IMX_MEDIA_GRP_ID_VIDMUX,
> >> +	IMX_MEDIA_GRP_ID_SENSOR,
> >> +	IMX_MEDIA_GRP_ID_CSI2,
> >> +};
> > This seems somewhat arbitrary. Why is a power sequence needed?
> 
> The CSI-2 receiver must be powered up before the sensor, that's the
> only requirement IIRC. The others have no s_power requirement. So I
> can probably change this to power up in the frontend -> backend order
> (IC_PP to sensor). And vice-versa for power off.

Yes, I think that should work (see below).

> > [...]
> >> +/*
> >> + * Turn current pipeline power on/off starting from start_entity.
> >> + * Must be called with mdev->graph_mutex held.
> >> + */
> >> +int imx_media_pipeline_set_power(struct imx_media_dev *imxmd,
> >> +				 struct media_entity_graph *graph,
> >> +				 struct media_entity *start_entity, bool on)
> >> +{
> >> +	struct media_entity *entity;
> >> +	struct v4l2_subdev *sd;
> >> +	int i, ret = 0;
> >> +	u32 id;
> >> +
> >> +	for (i = 0; i < NUM_POWER_ENTITIES; i++) {
> >> +		id = on ? power_on_seq[i] : power_off_seq[i];
> >> +		entity = find_pipeline_entity(imxmd, graph, start_entity, id);
> >> +		if (!entity)
> >> +			continue;
> >> +
> >> +		sd = media_entity_to_v4l2_subdev(entity);
> >> +
> >> +		ret = v4l2_subdev_call(sd, core, s_power, on);
> >> +		if (ret && ret != -ENOIOCTLCMD)
> >> +			break;
> >> +	}
> >> +
> >> +	return (ret && ret != -ENOIOCTLCMD) ? ret : 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(imx_media_pipeline_set_power);
> > This should really be handled by v4l2_pipeline_pm_use.
> 
> I thought about this earlier, but v4l2_pipeline_pm_use() seems to be
> doing some other stuff that bothered me, at least that's what I remember.
> I will revisit this.

I have used it with a tc358743 -> mipi-csi2 pipeline, it didn't cause
any problems. It would be better to reuse and, if necessary, fix the
existing infrastructure where available.

> >> +int imx_media_add_internal_subdevs(struct imx_media_dev *imxmd,
> >> +				   struct imx_media_subdev *csi[4])
> >> +{
> >> +	int ret;
> >> +
> >> +	/* there must be at least one CSI in first IPU */
> > Why?
> 
> Well yeah, imx-media doesn't necessarily need a CSI if things
> like the VDIC or post-processor are being used by an output
> overlay pipeline, for example. I'll fix this.

I haven't even thought that far, but there could be boards with only a
parallel sensor connected to IPU2 CSI1 and IPU1 disabled for power
saving reasons.

regards
Philipp

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web