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

Groups > linux.kernel > #1596626 > unrolled thread

[PATCH v5 00/39] i.MX Media Driver

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 26/39] media: imx: Add VDIC subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 38/39] media: imx: csi: fix crop rectangle reset in sink set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 30/39] media: imx: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 35/39] media: imx: csi/fim: add support for frame intervals Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 20/39] platform: add video-multiplexer subdevice driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 29/39] ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 09/39] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 10/39] ARM: dts: imx6-sabreauto: create i2cmux for i2c3 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 08/39] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 01/39] [media] dt-bindings: Add bindings for video-multiplexer device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 24/39] media: imx: Add Capture Device Interface Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 32/39] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 33/39] media: imx: mipi-csi2: enable setting and getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 25/39] media: imx: Add CSI subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 05/39] ARM: dts: imx6qdl: Add mipi_ipu1/2 multiplexers, mipi_csi, and their connections Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 11/39] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
      Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-11 14:50 +0100
        Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 19:30 +0100
          Re: [PATCH v5 21/39] UAPI: Add media UAPI Kbuild file Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 11:00 +0100
    [PATCH v5 37/39] media: imx: csi: add frame skipping support Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 22/39] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
      Re: [PATCH v5 22/39] media: Add userspace header file for i.MX Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 12:50 +0100
      Re: [PATCH v5 22/39] media: Add userspace header file for i.MX Pavel Machek <pavel@ucw.cz> - 2017-03-11 00:40 +0100
    [PATCH v5 36/39] media: imx: redo pixel format enumeration and negotiation Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
      Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 13:10 +0100
        Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 19:40 +0100
          Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Pavel Machek <pavel@ucw.cz> - 2017-03-11 00:40 +0100
            Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 00:50 +0100
          Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-11 12:50 +0100
            Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 19:20 +0100
              Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-11 20:00 +0100
                Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:00 +0100
                Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 20:10 +0100
              Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 11:10 +0100
                Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 11:50 +0100
                  Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 12:00 +0100
                    Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 18:10 +0100
                      Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-13 18:20 +0100
                        Re: [PATCH v5 15/39] [media] v4l2: add a frame interval error event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 22:50 +0100
    [PATCH v5 31/39] media: imx: csi: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 03/39] [media] dt/bindings: Add bindings for OV5640 Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:00 +0100
    [PATCH v5 14/39] add mux and video interface bridge entity functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
    [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
      Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum  ERR006687 workaround Troy Kisky <troy.kisky@boundarydevices.com> - 2017-03-10 20:00 +0100
        Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum  ERR006687 workaround Fabio Estevam <festevam@gmail.com> - 2017-03-10 20:20 +0100
          Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum  ERR006687 workaround Pavel Machek <pavel@ucw.cz> - 2017-03-10 23:00 +0100
            Re: [PATCH v5 07/39] ARM: dts: imx6qdl-sabrelite: remove erratum  ERR006687 workaround Fabio Estevam <festevam@gmail.com> - 2017-03-10 23:10 +0100
    [PATCH v5 17/39] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
      Re: [PATCH v5 17/39] [media] v4l2-mc: add a function to inherit  controls from a pipeline Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 12:50 +0100
    [PATCH v5 12/39] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
    [PATCH v5 18/39] [media] v4l: subdev: Add function to validate frame interval Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
      Re: [PATCH v5 18/39] [media] v4l: subdev: Add function to validate  frame interval Sakari Ailus <sakari.ailus@iki.fi> - 2017-03-11 14:50 +0100
        Re: [PATCH v5 18/39] [media] v4l: subdev: Add function to validate  frame interval Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 21:40 +0100
    [PATCH v5 04/39] ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
    [PATCH v5 16/39] [media] v4l2: add a new-frame before end-of-frame event Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
      Re: [PATCH v5 16/39] [media] v4l2: add a new-frame before  end-of-frame event Hans Verkuil <hverkuil@xs4all.nl> - 2017-03-10 13:10 +0100
    [PATCH v5 06/39] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
    [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-10 06:10 +0100
    Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-10 21:20 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-11 00:30 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 18:50 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 01:40 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:00 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:10 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:30 +0100
              Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-13 05:40 +0100
                Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 09:20 +0100
                  Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-13 10:40 +0100
                    Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-14 00:40 +0100
                  Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-03-14 00:40 +0100
    Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 19:00 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 20:30 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 20:40 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 20:50 +0100
            Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:20 +0100
              Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:40 +0100
                Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:40 +0100
                Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 21:50 +0100
                  Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 22:20 +0100
    Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 20:50 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-12 21:10 +0100
      Re: [PATCH v5 00/39] i.MX Media Driver Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 22:00 +0100
        Re: [PATCH v5 00/39] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-12 22:20 +0100
          Re: [PATCH v5 00/39] i.MX Media Driver Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-03-12 23:20 +0100

Page 4 of 5 — ← Prev page 1 2 3 [4] 5  Next page →

#1596660 — [PATCH v5 02/39] [media] dt-bindings: Add bindings for i.MX media driver

Add bindings documentation for the i.MX media driver.

Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
 Documentation/devicetree/bindings/media/imx.txt | 74 +++++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/imx.txt

diff --git a/Documentation/devicetree/bindings/media/imx.txt b/Documentation/devicetree/bindings/media/imx.txt
new file mode 100644
index 0000000..3059c06
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/imx.txt
@@ -0,0 +1,74 @@
+Freescale i.MX Media Video Device
+=================================
+
+Video Media Controller node
+---------------------------
+
+This is the media controller node for video capture support. It is a
+virtual device that lists the camera serial interface nodes that the
+media device will control.
+
+Required properties:
+- compatible : "fsl,imx-capture-subsystem";
+- ports      : Should contain a list of phandles pointing to camera
+		sensor interface ports of IPU devices
+
+example:
+
+capture-subsystem {
+	compatible = "fsl,imx-capture-subsystem";
+	ports = <&ipu1_csi0>, <&ipu1_csi1>;
+};
+
+fim child node
+--------------
+
+This is an optional child node of the ipu_csi port nodes. If present and
+available, it enables the Frame Interval Monitor. Its properties can be
+used to modify the method in which the FIM measures frame intervals.
+Refer to Documentation/media/v4l-drivers/imx.rst for more info on the
+Frame Interval Monitor.
+
+Optional properties:
+- fsl,input-capture-channel: an input capture channel and channel flags,
+			     specified as <chan flags>. The channel number
+			     must be 0 or 1. The flags can be
+			     IRQ_TYPE_EDGE_RISING, IRQ_TYPE_EDGE_FALLING, or
+			     IRQ_TYPE_EDGE_BOTH, and specify which input
+			     capture signal edge will trigger the input
+			     capture event. If an input capture channel is
+			     specified, the FIM will use this method to
+			     measure frame intervals instead of via the EOF
+			     interrupt. The input capture method is much
+			     preferred over EOF as it is not subject to
+			     interrupt latency errors. However it requires
+			     routing the VSYNC or FIELD output signals of
+			     the camera sensor to one of the i.MX input
+			     capture pads (SD1_DAT0, SD1_DAT1), which also
+			     gives up support for SD1.
+
+
+mipi_csi2 node
+--------------
+
+This is the device node for the MIPI CSI-2 Receiver, required for MIPI
+CSI-2 sensors.
+
+Required properties:
+- compatible	: "fsl,imx6-mipi-csi2", "snps,dw-mipi-csi2";
+- reg           : physical base address and length of the register set;
+- clocks	: the MIPI CSI-2 receiver requires three clocks: hsi_tx
+		  (the D-PHY clock), video_27m (D-PHY PLL reference
+		  clock), and eim_podf;
+- clock-names	: must contain "dphy", "ref", "pix";
+- port@*        : five port nodes must exist, containing endpoints
+		  connecting to the source and sink devices according to
+		  of_graph bindings. The first port is an input port,
+		  connecting with a MIPI CSI-2 source, and ports 1
+		  through 4 are output ports connecting with parallel
+		  bus sink endpoint nodes and correspond to the four
+		  MIPI CSI-2 virtual channel outputs.
+
+Optional properties:
+- interrupts	: must contain two level-triggered interrupts,
+		  in order: 100 and 101;
-- 
2.7.4

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

#1598070

Version 5 gives me no v4l2 controls exposed through the video device
interface.

Just like with version 4, version 5 is completely useless with IMX219:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
ipu1_csi0: pipeline start failed with -110
imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
ipu1_csi0: pipeline start failed with -110

So, like v4, I can't do any further testing.

On Thu, Mar 09, 2017 at 08:52:40PM -0800, Steve Longerbeam wrote:
> In version 5:
> 
> - ov5640: renamed "pwdn-gpios" to "powerdown-gpios"
> 
> - ov5640: add mutex lock around the subdev op entry points.
> 
> - ov5640: don't attempt to program the new mode in ov5640_set_fmt().
>   Instead set a new flag, pending_mode_change, and program the new
>   mode at s_stream() if flag is set.
> 
> - ov5640: implement [gs]_frame_interval. As part of that, create
>   ov5640_try_frame_interval(), which is used by both [gs]_frame_interval
>   and [gs]_parm.
> 
> - ov5640: don't attempt to set controls in ov5640_s_ctrl(), or at
>   mode change, do it instead after first power-up.
> 
> - video-multiplexer: include link_validate in media_entity_operations.
> 
> - video-multiplexer: enforce that output pad frame interval must match
>   input pad frame interval in vidsw_s_frame_interval().
> 
> - video-multiplexer: initialize frame interval to a default 30 fps.
> 
> - mipi csi-2: renamed "cfg" clock name property to "ref". This is the
>   27 MHz mipi csi-2 PLL reference clock.
> 
> - mipi csi-2: create a hsfreq_map[] table based on
>   https://community.nxp.com/docs/DOC-94312. Use it to select
>   a hsfreqrange_sel value when programming the D-PHY, based on
>   a max Mbps per lane. This is computed from the source subdev
>   via V4L2_CID_LINK_FREQ control, and if the subdev doesn't implement
>   that control, use a default hard-coded max Mbps per lane.
> 
> - added required ports property description to imx-media binding doc.
> 
> - removed event V4L2_EVENT_FRAME_TIMEOUT. On a frame timeout, which
>   is always unrecoverable, call vb2_queue_error() instead.
> 
> - export the remaining custom events to V4L2_EVENT_FRAME_INTERVAL_ERROR
>   and V4L2_EVENT_NEW_FRAME_BEFORE_EOF.
> 
> - vdic: use V4L2_CID_DEINTERLACING_MODE for motion compensation control
>   instead of a custom control.
> 
> - add v4l2_subdev_link_validate_frame_interval(). Call this in the
>   link_validate imx-media subdev callbacks and video-multiplexer.
> 
> - fix subdev event registration: implementation of subscribe_event()
>   and unsubscribe_event() subdev ops were missing.
> 
> - all calls from the pipeline to the sensor subdev have been removed.
>   Only the CSI subdev still refers to a sensor, and only to retrieve
>   its media bus config, which is necessary to setup the CSI interface.
> 
> - add mutex locks around the imx-media subdev op entry points.
> 
> - completed the propagation of all pad format parameters from sink
>   pads to source pads within every imx-media subdev.
> 
> - implement [gs]_frame_interval in all the imx-media subdevs.
> 
> - imx-ic-prpencvf: there isn't necessarily a CSI subdev in the pipeline
>   in the future, so make sure this is optional when calling the CSI's
>   FIM.
> 
> - the source pads that attach to capture device nodes now require the
>   IPU internal pixel codes. The capture device translates these to
>   v4l2 fourcc memory formats.
> 
> - fix control inheritance to the capture device. When the pipeline
>   was modified, the inherited controls were not being refreshed.
>   v4l2_pipeline_inherit_controls() is now called only in imx-media
>   link_notify() callback when a pipelink link is disabled or modified.
>   imx_media_find_pipeline_video_device() is created to locate the
>   capture device in the pipeline.
> 
> - fix a possible race when propagating formats to the capture device.
>   The subdevs and capture device use different mutex locks when setting
>   formats. imx_media_capture_device_set_format() is created which acquires
>   the capture device mutex when updating the capture device format.
> 
> - verify all subdevs were bound in the async completion callback.
>  
> 
> Philipp Zabel (7):
>   [media] dt-bindings: Add bindings for video-multiplexer device
>   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
>   media: imx: csi: fix crop rectangle changes in set_fmt
>   media: imx: csi: add frame skipping support
>   media: imx: csi: fix crop rectangle reset in sink set_fmt
> 
> Russell King (4):
>   media: imx: add support for bayer formats
>   media: imx: csi: add support for bayer formats
>   media: imx: mipi-csi2: enable setting and getting of frame rates
>   media: imx: csi/fim: add support for frame intervals
> 
> Steve Longerbeam (28):
>   [media] dt-bindings: Add bindings for i.MX media driver
>   [media] dt/bindings: Add bindings for OV5640
>   ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node
>   ARM: dts: imx6qdl: add capture-subsystem 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
>   [media] v4l2: add a frame interval error event
>   [media] v4l2: add a new-frame before end-of-frame event
>   [media] v4l2-mc: add a function to inherit controls from a pipeline
>   [media] v4l: subdev: Add function to validate frame interval
>   [media] add Omnivision OV5640 sensor driver
>   UAPI: Add media UAPI Kbuild file
>   media: Add userspace header file for i.MX
>   media: Add i.MX media core driver
>   media: imx: Add Capture Device Interface
>   media: imx: Add CSI subdev driver
>   media: imx: Add VDIC subdev driver
>   media: imx: Add IC subdev drivers
>   media: imx: Add MIPI CSI-2 Receiver subdev driver
>   ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers
>   media: imx: csi: add __csi_get_fmt
>   media: imx: redo pixel format enumeration and negotiation
>   media: imx: propagate sink pad formats to source pads
> 
>  .../devicetree/bindings/media/i2c/ov5640.txt       |   45 +
>  Documentation/devicetree/bindings/media/imx.txt    |   74 +
>  .../bindings/media/video-multiplexer.txt           |   59 +
>  Documentation/media/uapi/mediactl/media-types.rst  |   22 +
>  Documentation/media/uapi/v4l/vidioc-dqevent.rst    |   12 +
>  Documentation/media/v4l-drivers/imx.rst            |  560 +++++
>  Documentation/media/videodev2.h.rst.exceptions     |    2 +
>  arch/arm/boot/dts/imx6dl-sabrelite.dts             |    5 +
>  arch/arm/boot/dts/imx6dl-sabresd.dts               |    5 +
>  arch/arm/boot/dts/imx6dl.dtsi                      |  185 ++
>  arch/arm/boot/dts/imx6q-sabrelite.dts              |    5 +
>  arch/arm/boot/dts/imx6q-sabresd.dts                |    5 +
>  arch/arm/boot/dts/imx6q.dtsi                       |  121 ++
>  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi           |  144 +-
>  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi           |  152 +-
>  arch/arm/boot/dts/imx6qdl-sabresd.dtsi             |  114 +-
>  arch/arm/boot/dts/imx6qdl.dtsi                     |   17 +-
>  arch/arm/configs/imx_v6_v7_defconfig               |   11 +
>  drivers/media/i2c/Kconfig                          |    7 +
>  drivers/media/i2c/Makefile                         |    1 +
>  drivers/media/i2c/ov5640.c                         | 2231 ++++++++++++++++++++
>  drivers/media/platform/Kconfig                     |    8 +
>  drivers/media/platform/Makefile                    |    2 +
>  drivers/media/platform/video-multiplexer.c         |  498 +++++
>  drivers/media/v4l2-core/v4l2-mc.c                  |   48 +
>  drivers/media/v4l2-core/v4l2-subdev.c              |   50 +
>  drivers/staging/media/Kconfig                      |    2 +
>  drivers/staging/media/Makefile                     |    1 +
>  drivers/staging/media/imx/Kconfig                  |   20 +
>  drivers/staging/media/imx/Makefile                 |   12 +
>  drivers/staging/media/imx/TODO                     |   17 +
>  drivers/staging/media/imx/imx-ic-common.c          |  113 +
>  drivers/staging/media/imx/imx-ic-prp.c             |  497 +++++
>  drivers/staging/media/imx/imx-ic-prpencvf.c        | 1236 +++++++++++
>  drivers/staging/media/imx/imx-ic.h                 |   38 +
>  drivers/staging/media/imx/imx-media-capture.c      |  694 ++++++
>  drivers/staging/media/imx/imx-media-csi.c          | 1595 ++++++++++++++
>  drivers/staging/media/imx/imx-media-dev.c          |  522 +++++
>  drivers/staging/media/imx/imx-media-fim.c          |  463 ++++
>  drivers/staging/media/imx/imx-media-internal-sd.c  |  349 +++
>  drivers/staging/media/imx/imx-media-of.c           |  267 +++
>  drivers/staging/media/imx/imx-media-utils.c        | 1009 +++++++++
>  drivers/staging/media/imx/imx-media-vdic.c         |  949 +++++++++
>  drivers/staging/media/imx/imx-media.h              |  311 +++
>  drivers/staging/media/imx/imx6-mipi-csi2.c         |  725 +++++++
>  include/media/imx.h                                |   15 +
>  include/media/v4l2-mc.h                            |   25 +
>  include/media/v4l2-subdev.h                        |   10 +
>  include/uapi/Kbuild                                |    1 +
>  include/uapi/linux/media.h                         |    6 +
>  include/uapi/linux/v4l2-controls.h                 |    4 +
>  include/uapi/linux/videodev2.h                     |    2 +
>  include/uapi/media/Kbuild                          |    2 +
>  include/uapi/media/imx.h                           |   21 +
>  54 files changed, 13262 insertions(+), 27 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov5640.txt
>  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/i2c/ov5640.c
>  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-ic-common.c
>  create mode 100644 drivers/staging/media/imx/imx-ic-prp.c
>  create mode 100644 drivers/staging/media/imx/imx-ic-prpencvf.c
>  create mode 100644 drivers/staging/media/imx/imx-ic.h
>  create mode 100644 drivers/staging/media/imx/imx-media-capture.c
>  create mode 100644 drivers/staging/media/imx/imx-media-csi.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-utils.c
>  create mode 100644 drivers/staging/media/imx/imx-media-vdic.c
>  create mode 100644 drivers/staging/media/imx/imx-media.h
>  create mode 100644 drivers/staging/media/imx/imx6-mipi-csi2.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
> 

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598172

On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:
> Version 5 gives me no v4l2 controls exposed through the video device
> interface.
>
> Just like with version 4, version 5 is completely useless with IMX219:
>
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
>
> So, like v4, I can't do any further testing.
>

Is the imx219 placing the csi-2 bus in LP-11 state on exit
from s_power(ON)?

I realize that probably means bringing the chip up to a
completely operational state and then setting it to stream
OFF in the s_power() op.

The same had to be done for the OV5640.

Steve

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

#1598698

On Fri, Mar 10, 2017 at 03:20:34PM -0800, Steve Longerbeam wrote:
> 
> 
> On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:
> >Version 5 gives me no v4l2 controls exposed through the video device
> >interface.
> >
> >Just like with version 4, version 5 is completely useless with IMX219:
> >
> >imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> >ipu1_csi0: pipeline start failed with -110
> >imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> >ipu1_csi0: pipeline start failed with -110
> >imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> >ipu1_csi0: pipeline start failed with -110
> >
> >So, like v4, I can't do any further testing.
> >
> 
> Is the imx219 placing the csi-2 bus in LP-11 state on exit
> from s_power(ON)?
> 
> I realize that probably means bringing the chip up to a
> completely operational state and then setting it to stream
> OFF in the s_power() op.
> 
> The same had to be done for the OV5640.

What do you suggest - setting it to the highest CSI2 bus speed that
it supports?  That's likely to be over the maximum data rate specified
for iMX6Q if it's wired up using four lanes.


Also, as I've already said, I think that powering on the sensor just
because it's got an enabled media-controller link is a silly idea.

Right now, the only way of using the imx6 capture stuff is to manually
configure it with media-ctl, which means that happens either at boot
due to a custom boot script, or when you first use it (by manually
running a script.)

This results in the sensor staying powered from that point onwards,
wasting power unnecessarily.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598516

[Multipart message — attachments visible in raw view] — view raw

On 03/10/2017 12:13 PM, Russell King - ARM Linux wrote:
> Version 5 gives me no v4l2 controls exposed through the video device
> interface.
>
> Just like with version 4, version 5 is completely useless with IMX219:
>
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> ipu1_csi0: pipeline start failed with -110
>

If it's too difficult to get the imx219 csi-2 transmitter into the
LP-11 state on power on, perhaps the csi-2 receiver can be a little
more lenient on the transmitter and make the LP-11 timeout a warning
instead of error-out.

Can you try the attached change on top of the version 5 patchset?

If that doesn't work then you're just going to have to fix the bug
in imx219.

Steve

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

#1598728

On Sat, Mar 11, 2017 at 04:30:53PM -0800, Steve Longerbeam wrote:
> If it's too difficult to get the imx219 csi-2 transmitter into the
> LP-11 state on power on, perhaps the csi-2 receiver can be a little
> more lenient on the transmitter and make the LP-11 timeout a warning
> instead of error-out.
> 
> Can you try the attached change on top of the version 5 patchset?
> 
> If that doesn't work then you're just going to have to fix the bug
> in imx219.

That patch gets me past that hurdle, only to reveal that there's another
issue:

imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
imx219 0-0010: VT: pixclk 139200000Hz line 80742Hz frame 30.0Hz
imx219 0-0010: VT: line period 12385ns
imx219 0-0010: OP: pixclk 38500000Hz, 2 lanes, 308Mbps peak each
imx219 0-0010: OP: 3288 bits/line/lane act=10675ns lp/idle=1710ns
ipu1_csi0: csi_idmac_setup failed: -22
ipu1_csi0: pipeline start failed with -22
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1860 at /home/rmk/git/linux-rmk/drivers/media/v4l2-core/videobuf2-core.c:1340 vb2_start_streaming+0x124/0x1b4 [videobuf2_core]

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598735

On 03/12/2017 12:57 PM, Russell King - ARM Linux wrote:
> On Sat, Mar 11, 2017 at 04:30:53PM -0800, Steve Longerbeam wrote:
>> If it's too difficult to get the imx219 csi-2 transmitter into the
>> LP-11 state on power on, perhaps the csi-2 receiver can be a little
>> more lenient on the transmitter and make the LP-11 timeout a warning
>> instead of error-out.
>>
>> Can you try the attached change on top of the version 5 patchset?
>>
>> If that doesn't work then you're just going to have to fix the bug
>> in imx219.
>
> That patch gets me past that hurdle, only to reveal that there's another
> issue:

Yeah, ipu_cpmem_set_image() failed because it doesn't recognize the
bayer formats. Wait, didn't we fix this already? I've lost track.
Ah, right, we were going to move this support into the IPUv3 driver,
but in the meantime I think you had some patches to get around this.

Steve


>
> imx6-mipi-csi2: LP-11 timeout, phy_state = 0x00000200
> imx219 0-0010: VT: pixclk 139200000Hz line 80742Hz frame 30.0Hz
> imx219 0-0010: VT: line period 12385ns
> imx219 0-0010: OP: pixclk 38500000Hz, 2 lanes, 308Mbps peak each
> imx219 0-0010: OP: 3288 bits/line/lane act=10675ns lp/idle=1710ns
> ipu1_csi0: csi_idmac_setup failed: -22
> ipu1_csi0: pipeline start failed with -22
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1860 at /home/rmk/git/linux-rmk/drivers/media/v4l2-core/videobuf2-core.c:1340 vb2_start_streaming+0x124/0x1b4 [videobuf2_core]
>

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

#1598741

On Sun, Mar 12, 2017 at 01:05:06PM -0700, Steve Longerbeam wrote:
> 
> 
> On 03/12/2017 12:57 PM, Russell King - ARM Linux wrote:
> >On Sat, Mar 11, 2017 at 04:30:53PM -0800, Steve Longerbeam wrote:
> >>If it's too difficult to get the imx219 csi-2 transmitter into the
> >>LP-11 state on power on, perhaps the csi-2 receiver can be a little
> >>more lenient on the transmitter and make the LP-11 timeout a warning
> >>instead of error-out.
> >>
> >>Can you try the attached change on top of the version 5 patchset?
> >>
> >>If that doesn't work then you're just going to have to fix the bug
> >>in imx219.
> >
> >That patch gets me past that hurdle, only to reveal that there's another
> >issue:
> 
> Yeah, ipu_cpmem_set_image() failed because it doesn't recognize the
> bayer formats. Wait, didn't we fix this already? I've lost track.
> Ah, right, we were going to move this support into the IPUv3 driver,
> but in the meantime I think you had some patches to get around this.

What I had was this patch for your v3.  I never got to testing your
v4 because of the LP-11 problem.

In v5, you've changed to propagate the ipu_cpmem_set_image() error
code to avoid the resulting corruption, but that leaves the other bits
of this patch unaddressed, along my "media: imx: smfc: add support
for bayer formats" patch.

Your driver basically has no support for bayer formats.

diff --git a/drivers/staging/media/imx/imx-smfc.c b/drivers/staging/media/imx/imx-smfc.c
index 313732201a52..4351c0365cf4 100644
--- a/drivers/staging/media/imx/imx-smfc.c
+++ b/drivers/staging/media/imx/imx-smfc.c
@@ -234,11 +234,6 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
 	buf1 = imx_media_dma_buf_get_next_queued(priv->out_ring);
 	priv->next = buf1;
 
-	image.phys0 = buf0->phys;
-	image.phys1 = buf1->phys;
-	ipu_cpmem_set_image(priv->smfc_ch, &image);
-
-
 	switch (image.pix.pixelformat) {
 	case V4L2_PIX_FMT_SBGGR8:
 	case V4L2_PIX_FMT_SGBRG8:
@@ -247,6 +242,10 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
 		burst_size = 8;
 		passthrough = true;
 		passthrough_bits = 8;
+		ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height);
+		ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline);
+		ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys);
+		ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys);
 		break;
 
 	case V4L2_PIX_FMT_SBGGR16:
@@ -256,9 +255,17 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
 		burst_size = 4;
 		passthrough = true;
 		passthrough_bits = 16;
+		ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height);
+		ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline);
+		ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys);
+		ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys);
 		break;
 
 	default:
+		image.phys0 = buf0->phys;
+		image.phys1 = buf1->phys;
+		ipu_cpmem_set_image(priv->smfc_ch, &image);
+
 		burst_size = (outfmt->width & 0xf) ? 8 : 16;
 
 		/*

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598844

On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 01:05:06PM -0700, Steve Longerbeam wrote:
>>
>>
>> On 03/12/2017 12:57 PM, Russell King - ARM Linux wrote:
>>> On Sat, Mar 11, 2017 at 04:30:53PM -0800, Steve Longerbeam wrote:
>>>> If it's too difficult to get the imx219 csi-2 transmitter into the
>>>> LP-11 state on power on, perhaps the csi-2 receiver can be a little
>>>> more lenient on the transmitter and make the LP-11 timeout a warning
>>>> instead of error-out.
>>>>
>>>> Can you try the attached change on top of the version 5 patchset?
>>>>
>>>> If that doesn't work then you're just going to have to fix the bug
>>>> in imx219.
>>>
>>> That patch gets me past that hurdle, only to reveal that there's another
>>> issue:
>>
>> Yeah, ipu_cpmem_set_image() failed because it doesn't recognize the
>> bayer formats. Wait, didn't we fix this already? I've lost track.
>> Ah, right, we were going to move this support into the IPUv3 driver,
>> but in the meantime I think you had some patches to get around this.
>
> What I had was this patch for your v3.  I never got to testing your
> v4 because of the LP-11 problem.
>
> In v5, you've changed to propagate the ipu_cpmem_set_image() error
> code to avoid the resulting corruption, but that leaves the other bits
> of this patch unaddressed, along my "media: imx: smfc: add support
> for bayer formats" patch.
>
> Your driver basically has no support for bayer formats.

You added the patches to this driver that adds the bayer support,
I don't think there is anything more required of the driver at this
point to support bayer, the remaining work needs to happen in the IPUv3
driver.

I'll see if I have time to write that patch to IPUv3, but it's simple,
in fact what you wrote below can be translate directly into
ipu_cpmem_set_image(). There's a few other places bayer needs to be
treated in IPUv3, but it should be obvious by grepping for the
reference to pixel formats.

Steve


>
> diff --git a/drivers/staging/media/imx/imx-smfc.c b/drivers/staging/media/imx/imx-smfc.c
> index 313732201a52..4351c0365cf4 100644
> --- a/drivers/staging/media/imx/imx-smfc.c
> +++ b/drivers/staging/media/imx/imx-smfc.c
> @@ -234,11 +234,6 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
>  	buf1 = imx_media_dma_buf_get_next_queued(priv->out_ring);
>  	priv->next = buf1;
>
> -	image.phys0 = buf0->phys;
> -	image.phys1 = buf1->phys;
> -	ipu_cpmem_set_image(priv->smfc_ch, &image);
> -
> -
>  	switch (image.pix.pixelformat) {
>  	case V4L2_PIX_FMT_SBGGR8:
>  	case V4L2_PIX_FMT_SGBRG8:
> @@ -247,6 +242,10 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
>  		burst_size = 8;
>  		passthrough = true;
>  		passthrough_bits = 8;
> +		ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height);
> +		ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline);
> +		ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys);
> +		ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys);
>  		break;
>
>  	case V4L2_PIX_FMT_SBGGR16:
> @@ -256,9 +255,17 @@ static void imx_smfc_setup_channel(struct imx_smfc_priv *priv)
>  		burst_size = 4;
>  		passthrough = true;
>  		passthrough_bits = 16;
> +		ipu_cpmem_set_resolution(priv->smfc_ch, image.rect.width, image.rect.height);
> +		ipu_cpmem_set_stride(priv->smfc_ch, image.pix.bytesperline);
> +		ipu_cpmem_set_buffer(priv->smfc_ch, 0, buf0->phys);
> +		ipu_cpmem_set_buffer(priv->smfc_ch, 1, buf1->phys);
>  		break;
>
>  	default:
> +		image.phys0 = buf0->phys;
> +		image.phys1 = buf1->phys;
> +		ipu_cpmem_set_image(priv->smfc_ch, &image);
> +
>  		burst_size = (outfmt->width & 0xf) ? 8 : 16;
>
>  		/*
>

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

#1598929

On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:
> On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
> >What I had was this patch for your v3.  I never got to testing your
> >v4 because of the LP-11 problem.
> >
> >In v5, you've changed to propagate the ipu_cpmem_set_image() error
> >code to avoid the resulting corruption, but that leaves the other bits
> >of this patch unaddressed, along my "media: imx: smfc: add support
> >for bayer formats" patch.
> >
> >Your driver basically has no support for bayer formats.
> 
> You added the patches to this driver that adds the bayer support,
> I don't think there is anything more required of the driver at this
> point to support bayer, the remaining work needs to happen in the IPUv3
> driver.

There is more work, because the way you've merged my changes to
imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
respect to the burst size.

You always set it to 8 or 16 depending on the width:

	burst_size = (image.pix.width & 0xf) ? 8 : 16;

	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);

and then you have my switch() statement which assigns burst_size.
My _tested_ code removed the above, added the switch, which had
a default case which reflected the above setting:

	default:
		burst_size = (outfmt->width & 0xf) ? 8 : 16;

and then went on to set the burst size _after_ the switch statement:

	ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);

The effect is unchanged for non-bayer formats.  For bayer formats, the
burst size is determined by the bayer data size.

So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
above is still required.

I'm not convinced that fixing ipu_cpmem_set_image() is even the best
solution - it's not as trivial as it looks on the surface:

        ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
        ipu_cpmem_set_stride(ch, pix->bytesperline);

this is fine, it doesn't depend on the format.  However, the next line:

        ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));

does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
DRM knows nothing about bayer formats, there aren't fourcc codes in
DRM for it.  The result is that v4l2_pix_fmt_to_drm_fourcc() returns
-EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().

ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
it's a bug that this is not checked and propagated.  If it is checked and
propagated, then we need this to support bayer formats, and I don't see
DRM people wanting bayer format fourcc codes added without there being
a real DRM driver wanting to use them.

Then there's the business of calculating the top-left offset of the image,
which for bayer always needs to be an even number of pixels - as this
function takes the top-left offset, it ought to respect it, but if it
doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
always sets them to zero, but that's not really something that
ipu_cpmem_set_image() should assume.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1599146

On Mon, Mar 13, 2017 at 08:16:25AM +0000, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:
> > On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
> > >What I had was this patch for your v3.  I never got to testing your
> > >v4 because of the LP-11 problem.
> > >
> > >In v5, you've changed to propagate the ipu_cpmem_set_image() error
> > >code to avoid the resulting corruption, but that leaves the other bits
> > >of this patch unaddressed, along my "media: imx: smfc: add support
> > >for bayer formats" patch.
> > >
> > >Your driver basically has no support for bayer formats.
> > 
> > You added the patches to this driver that adds the bayer support,
> > I don't think there is anything more required of the driver at this
> > point to support bayer, the remaining work needs to happen in the IPUv3
> > driver.
> 
> There is more work, because the way you've merged my changes to
> imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
> respect to the burst size.
> 
> You always set it to 8 or 16 depending on the width:
> 
> 	burst_size = (image.pix.width & 0xf) ? 8 : 16;
> 
> 	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
> 
> and then you have my switch() statement which assigns burst_size.
> My _tested_ code removed the above, added the switch, which had
> a default case which reflected the above setting:
> 
> 	default:
> 		burst_size = (outfmt->width & 0xf) ? 8 : 16;
> 
> and then went on to set the burst size _after_ the switch statement:
> 
> 	ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);
> 
> The effect is unchanged for non-bayer formats.  For bayer formats, the
> burst size is determined by the bayer data size.
> 
> So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
> above is still required.
> 
> I'm not convinced that fixing ipu_cpmem_set_image() is even the best
> solution - it's not as trivial as it looks on the surface:
> 
>         ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
>         ipu_cpmem_set_stride(ch, pix->bytesperline);
> 
> this is fine, it doesn't depend on the format.  However, the next line:
> 
>         ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));
> 
> does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
> isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
> DRM knows nothing about bayer formats, there aren't fourcc codes in
> DRM for it.  The result is that v4l2_pix_fmt_to_drm_fourcc() returns
> -EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().
> 
> ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
> it's a bug that this is not checked and propagated.  If it is checked and
> propagated, then we need this to support bayer formats, and I don't see
> DRM people wanting bayer format fourcc codes added without there being
> a real DRM driver wanting to use them.
> 
> Then there's the business of calculating the top-left offset of the image,
> which for bayer always needs to be an even number of pixels - as this
> function takes the top-left offset, it ought to respect it, but if it
> doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
> always sets them to zero, but that's not really something that
> ipu_cpmem_set_image() should assume.

For the time being, I've restored the functionality along the same lines
as I originally had.  This seems to get me working capture, but might
break non-bayer passthrough mode:

diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index fc0036aa84d0..df336971a009 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -314,14 +314,6 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 	image.phys0 = phys[0];
 	image.phys1 = phys[1];
 
-	ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
-	if (ret)
-		return ret;
-
-	burst_size = (image.pix.width & 0xf) ? 8 : 16;
-
-	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
-
 	/*
 	 * Check for conditions that require the IPU to handle the
 	 * data internally as generic data, aka passthrough mode:
@@ -346,15 +338,29 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 		passthrough_bits = 16;
 		break;
 	default:
+		burst_size = (image.pix.width & 0xf) ? 8 : 16;
 		passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
 			       sensor_ep->bus.parallel.bus_width >= 16);
 		passthrough_bits = 16;
 		break;
 	}
 
-	if (passthrough)
+	if (passthrough) {
+		ipu_cpmem_set_resolution(priv->idmac_ch, image.rect.width,
+					 image.rect.height);
+		ipu_cpmem_set_stride(priv->idmac_ch, image.pix.bytesperline);
+		ipu_cpmem_set_buffer(priv->idmac_ch, 0, image.phys0);
+		ipu_cpmem_set_buffer(priv->idmac_ch, 1, image.phys1);
+		ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
 		ipu_cpmem_set_format_passthrough(priv->idmac_ch,
 						 passthrough_bits);
+	} else {
+		ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
+		if (ret)
+			return ret;
+
+		ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
+	}
 
 	/*
 	 * Set the channel for the direct CSI-->memory via SMFC


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1599921

er, I meant I will integrate this patch. And verify/fix
possible breakage for non-bayer passthrough.

Steve


On 03/13/2017 02:30 AM, Russell King - ARM Linux wrote:
> On Mon, Mar 13, 2017 at 08:16:25AM +0000, Russell King - ARM Linux wrote:
>> On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:
>>> On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
>>>> What I had was this patch for your v3.  I never got to testing your
>>>> v4 because of the LP-11 problem.
>>>>
>>>> In v5, you've changed to propagate the ipu_cpmem_set_image() error
>>>> code to avoid the resulting corruption, but that leaves the other bits
>>>> of this patch unaddressed, along my "media: imx: smfc: add support
>>>> for bayer formats" patch.
>>>>
>>>> Your driver basically has no support for bayer formats.
>>> You added the patches to this driver that adds the bayer support,
>>> I don't think there is anything more required of the driver at this
>>> point to support bayer, the remaining work needs to happen in the IPUv3
>>> driver.
>> There is more work, because the way you've merged my changes to
>> imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
>> respect to the burst size.
>>
>> You always set it to 8 or 16 depending on the width:
>>
>> 	burst_size = (image.pix.width & 0xf) ? 8 : 16;
>>
>> 	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
>>
>> and then you have my switch() statement which assigns burst_size.
>> My _tested_ code removed the above, added the switch, which had
>> a default case which reflected the above setting:
>>
>> 	default:
>> 		burst_size = (outfmt->width & 0xf) ? 8 : 16;
>>
>> and then went on to set the burst size _after_ the switch statement:
>>
>> 	ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);
>>
>> The effect is unchanged for non-bayer formats.  For bayer formats, the
>> burst size is determined by the bayer data size.
>>
>> So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
>> above is still required.
>>
>> I'm not convinced that fixing ipu_cpmem_set_image() is even the best
>> solution - it's not as trivial as it looks on the surface:
>>
>>          ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
>>          ipu_cpmem_set_stride(ch, pix->bytesperline);
>>
>> this is fine, it doesn't depend on the format.  However, the next line:
>>
>>          ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));
>>
>> does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
>> isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
>> DRM knows nothing about bayer formats, there aren't fourcc codes in
>> DRM for it.  The result is that v4l2_pix_fmt_to_drm_fourcc() returns
>> -EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().
>>
>> ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
>> it's a bug that this is not checked and propagated.  If it is checked and
>> propagated, then we need this to support bayer formats, and I don't see
>> DRM people wanting bayer format fourcc codes added without there being
>> a real DRM driver wanting to use them.
>>
>> Then there's the business of calculating the top-left offset of the image,
>> which for bayer always needs to be an even number of pixels - as this
>> function takes the top-left offset, it ought to respect it, but if it
>> doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
>> always sets them to zero, but that's not really something that
>> ipu_cpmem_set_image() should assume.
> For the time being, I've restored the functionality along the same lines
> as I originally had.  This seems to get me working capture, but might
> break non-bayer passthrough mode:
>
> diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
> index fc0036aa84d0..df336971a009 100644
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -314,14 +314,6 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   	image.phys0 = phys[0];
>   	image.phys1 = phys[1];
>   
> -	ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
> -	if (ret)
> -		return ret;
> -
> -	burst_size = (image.pix.width & 0xf) ? 8 : 16;
> -
> -	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
> -
>   	/*
>   	 * Check for conditions that require the IPU to handle the
>   	 * data internally as generic data, aka passthrough mode:
> @@ -346,15 +338,29 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   		passthrough_bits = 16;
>   		break;
>   	default:
> +		burst_size = (image.pix.width & 0xf) ? 8 : 16;
>   		passthrough = (sensor_ep->bus_type != V4L2_MBUS_CSI2 &&
>   			       sensor_ep->bus.parallel.bus_width >= 16);
>   		passthrough_bits = 16;
>   		break;
>   	}
>   
> -	if (passthrough)
> +	if (passthrough) {
> +		ipu_cpmem_set_resolution(priv->idmac_ch, image.rect.width,
> +					 image.rect.height);
> +		ipu_cpmem_set_stride(priv->idmac_ch, image.pix.bytesperline);
> +		ipu_cpmem_set_buffer(priv->idmac_ch, 0, image.phys0);
> +		ipu_cpmem_set_buffer(priv->idmac_ch, 1, image.phys1);
> +		ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
>   		ipu_cpmem_set_format_passthrough(priv->idmac_ch,
>   						 passthrough_bits);
> +	} else {
> +		ret = ipu_cpmem_set_image(priv->idmac_ch, &image);
> +		if (ret)
> +			return ret;
> +
> +		ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
> +	}
>   
>   	/*
>   	 * Set the channel for the direct CSI-->memory via SMFC
>
>

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

#1599920

On 03/13/2017 01:16 AM, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 09:26:41PM -0700, Steve Longerbeam wrote:
>> On 03/12/2017 01:22 PM, Russell King - ARM Linux wrote:
>>> What I had was this patch for your v3.  I never got to testing your
>>> v4 because of the LP-11 problem.
>>>
>>> In v5, you've changed to propagate the ipu_cpmem_set_image() error
>>> code to avoid the resulting corruption, but that leaves the other bits
>>> of this patch unaddressed, along my "media: imx: smfc: add support
>>> for bayer formats" patch.
>>>
>>> Your driver basically has no support for bayer formats.
>> You added the patches to this driver that adds the bayer support,
>> I don't think there is anything more required of the driver at this
>> point to support bayer, the remaining work needs to happen in the IPUv3
>> driver.
> There is more work, because the way you've merged my changes to
> imx_smfc_setup_channel() into csi_idmac_setup_channel() is wrong with
> respect to the burst size.
>
> You always set it to 8 or 16 depending on the width:
>
> 	burst_size = (image.pix.width & 0xf) ? 8 : 16;
>
> 	ipu_cpmem_set_burstsize(priv->idmac_ch, burst_size);
>
> and then you have my switch() statement which assigns burst_size.
> My _tested_ code removed the above, added the switch, which had
> a default case which reflected the above setting:
>
> 	default:
> 		burst_size = (outfmt->width & 0xf) ? 8 : 16;
>
> and then went on to set the burst size _after_ the switch statement:
>
> 	ipu_cpmem_set_burstsize(priv->smfc_ch, burst_size);
>
> The effect is unchanged for non-bayer formats.  For bayer formats, the
> burst size is determined by the bayer data size.
>
> So, even if it's appropriate to fix ipu_cpmem_set_image(), fixing the
> above is still required.

Oops, sorry missed that. I'll fix.

>
> I'm not convinced that fixing ipu_cpmem_set_image() is even the best
> solution - it's not as trivial as it looks on the surface:
>
>          ipu_cpmem_set_resolution(ch, image->rect.width, image->rect.height);
>          ipu_cpmem_set_stride(ch, pix->bytesperline);
>
> this is fine, it doesn't depend on the format.  However, the next line:
>
>          ipu_cpmem_set_fmt(ch, v4l2_pix_fmt_to_drm_fourcc(pix->pixelformat));
>
> does - v4l2_pix_fmt_to_drm_fourcc() is a locally defined function (it
> isn't v4l2 code) that converts a v4l2 pixel format to a DRM fourcc.
> DRM knows nothing about bayer formats, there aren't fourcc codes in
> DRM for it.

right, yeah that's a problem.

>    The result is that v4l2_pix_fmt_to_drm_fourcc() returns
> -EINVAL cast to a u32, which gets passed unchecked into ipu_cpmem_set_fmt().

Ugh.

>
> ipu_cpmem_set_fmt() won't recognise that, and also returns -EINVAL - and
> it's a bug that this is not checked and propagated.  If it is checked and
> propagated, then we need this to support bayer formats, and I don't see
> DRM people wanting bayer format fourcc codes added without there being
> a real DRM driver wanting to use them.

true.

>
> Then there's the business of calculating the top-left offset of the image,
> which for bayer always needs to be an even number of pixels - as this
> function takes the top-left offset, it ought to respect it, but if it
> doesn't meet this criteria, what should it do?  csi_idmac_setup_channel()
> always sets them to zero, but that's not really something that
> ipu_cpmem_set_image() should assume.

Well, I will integrate your patch above. Thanks for doing this
work for me.

We do need to address the issues you brought up in ipu_cpmem at
some point.

Steve

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

#1598703

I've just looked at my test system's dmesg, and spotted this in the log.
It's been a while since these popped out of the kernel, so I don't know
what caused them (other than the obvious, a media-ctl command.)

My script which sets this up only enables links, and then configures the
formats etc, and doesn't disable them, so I don't see why the power
count should be going negative.

------------[ cut here ]------------
WARNING: CPU: 1 PID: 1889 at drivers/staging/media/imx/imx-media-csi.c:806 csi_s_power+0x9c/0xa8 [imx_media_csi]
Modules linked in: caam_jr uvcvideo snd_soc_imx_sgtl5000 snd_soc_fsl_asoc_card snd_soc_imx_spdif imx_media_csi(C) imx6_mipi_csi2(C) snd_soc_imx_audmux snd_soc_sgtl5000 imx219 imx_media_ic(C) imx_media_capture(C) imx_media_vdic(C) caam video_multiplexer imx_sdma coda v4l2_mem2mem videobuf2_v4l2 imx2_wdt imx_vdoa videobuf2_dma_contig videobuf2_core videobuf2_vmalloc videobuf2_memops snd_soc_fsl_ssi
imx_thermal snd_soc_fsl_spdif imx_pcm_dma imx_media(C) imx_media_common(C) nfsd
rc_pinnacle_pctv_hd dw_hdmi_ahb_audio dw_hdmi_cec etnaviv
CPU: 1 PID: 1889 Comm: media-ctl Tainted: G         C      4.11.0-rc1+ #2125
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013ba4>] (dump_backtrace) from [<c0013de4>] (show_stack+0x18/0x1c)
 r6:600e0013 r5:ffffffff r4:00000000 r3:00000000
[<c0013dcc>] (show_stack) from [<c033f728>] (dump_stack+0xa4/0xdc)
[<c033f684>] (dump_stack) from [<c00326cc>] (__warn+0xdc/0x108)
 r6:bf124014 r5:00000000 r4:00000000 r3:c09ea4a8
[<c00325f0>] (__warn) from [<c0032720>] (warn_slowpath_null+0x28/0x30)
 r10:ede00010 r8:ede4a348 r7:d039501c r6:d0395140 r5:00000000 r4:d0395010
[<c00326f8>] (warn_slowpath_null) from [<bf1229e0>] (csi_s_power+0x9c/0xa8 [imx_media_csi])
[<bf122944>] (csi_s_power [imx_media_csi]) from [<bf04a930>] (imx_media_set_power+0x3c/0x108 [imx_media_common])
 r7:d039501c r6:00000000 r5:00000000 r4:0000000c
[<bf04a8f4>] (imx_media_set_power [imx_media_common]) from [<bf04aa34>] (imx_media_pipeline_set_power+0x38/0x40 [imx_media_common])
 r10:00000001 r9:00000001 r8:ede4a348 r7:ede00010 r6:ede4a348 r5:d039501c
 r4:00000001
[<bf04a9fc>] (imx_media_pipeline_set_power [imx_media_common]) from [<bf052148>] (imx_media_link_notify+0xf0/0x144 [imx_media])
 r7:ede00010 r6:ed59f900 r5:00000000 r4:d039501c
[<bf052058>] (imx_media_link_notify [imx_media]) from [<c04fa858>] (__media_entity_setup_link+0x110/0x1d8)
 r10:c0347c03 r9:d7eb3dc8 r8:befe92b0 r7:ede00010 r6:00000000 r5:00000001
 r4:ed59f900 r3:bf052058
[<c04fa748>] (__media_entity_setup_link) from [<c04f9bb0>] (media_device_setup_link+0x84/0x90)
 r7:ede00010 r6:ede00010 r5:ef3fd810 r4:d7eb3dc8
[<c04f9b2c>] (media_device_setup_link) from [<c04f9e94>] (media_device_ioctl+0xa4/0x148)
 r6:00000000 r5:d7eb3dc8 r4:c077b014 r3:c04f9b2c
[<c04f9df0>] (media_device_ioctl) from [<c04fa3a0>] (media_ioctl+0x38/0x4c)
 r10:ed5eca68 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:00000003 r5:e82ca280
 r4:c0190304
[<c04fa368>] (media_ioctl) from [<c018f9c0>] (do_vfs_ioctl+0x98/0x9a0)
[<c018f928>] (do_vfs_ioctl) from [<c0190304>] (SyS_ioctl+0x3c/0x60)
 r10:00000000 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:c0347c03 r5:e82ca280
 r4:e82ca280
[<c01902c8>] (SyS_ioctl) from [<c000fd60>] (ret_fast_syscall+0x0/0x1c)
 r8:c000ff04 r7:00000036 r6:000261d0 r5:00000001 r4:009162e8 r3:00000001
---[ end trace 4fdd40e5adfc4485 ]---
------------[ cut here ]------------
WARNING: CPU: 1 PID: 1889 at drivers/staging/media/imx/imx-media-csi.c:806 csi_s_power+0x9c/0xa8 [imx_media_csi]
Modules linked in: caam_jr uvcvideo snd_soc_imx_sgtl5000 snd_soc_fsl_asoc_card snd_soc_imx_spdif imx_media_csi(C) imx6_mipi_csi2(C) snd_soc_imx_audmux snd_soc_sgtl5000 imx219 imx_media_ic(C) imx_media_capture(C) imx_media_vdic(C) caam video_multiplexer imx_sdma coda v4l2_mem2mem videobuf2_v4l2 imx2_wdt imx_vdoa videobuf2_dma_contig videobuf2_core videobuf2_vmalloc videobuf2_memops snd_soc_fsl_ssi
imx_thermal snd_soc_fsl_spdif imx_pcm_dma imx_media(C) imx_media_common(C) nfsd
rc_pinnacle_pctv_hd dw_hdmi_ahb_audio dw_hdmi_cec etnaviv
CPU: 1 PID: 1889 Comm: media-ctl Tainted: G        WC      4.11.0-rc1+ #2125
Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
Backtrace:
[<c0013ba4>] (dump_backtrace) from [<c0013de4>] (show_stack+0x18/0x1c)
 r6:600e0013 r5:ffffffff r4:00000000 r3:00000000
[<c0013dcc>] (show_stack) from [<c033f728>] (dump_stack+0xa4/0xdc)
[<c033f684>] (dump_stack) from [<c00326cc>] (__warn+0xdc/0x108)
 r6:bf124014 r5:00000000 r4:00000000 r3:c09ea4a8
[<c00325f0>] (__warn) from [<c0032720>] (warn_slowpath_null+0x28/0x30)
 r10:ede00010 r8:ede4a348 r7:ee000800 r6:d0395140 r5:00000000 r4:d0395010
[<c00326f8>] (warn_slowpath_null) from [<bf1229e0>] (csi_s_power+0x9c/0xa8 [imx_media_csi])
[<bf122944>] (csi_s_power [imx_media_csi]) from [<bf04a930>] (imx_media_set_power+0x3c/0x108 [imx_media_common])
 r7:ee000800 r6:00000000 r5:00000000 r4:0000000c
[<bf04a8f4>] (imx_media_set_power [imx_media_common]) from [<bf04aa34>] (imx_media_pipeline_set_power+0x38/0x40 [imx_media_common])
 r10:00000001 r9:00000001 r8:ede4a348 r7:ede00010 r6:ede4a348 r5:ee000800
 r4:00000001
[<bf04a9fc>] (imx_media_pipeline_set_power [imx_media_common]) from [<bf052148>] (imx_media_link_notify+0xf0/0x144 [imx_media])
 r7:ede00010 r6:d0320480 r5:ee000800 r4:ee000800
[<bf052058>] (imx_media_link_notify [imx_media]) from [<c04fa858>] (__media_entity_setup_link+0x110/0x1d8)
 r10:c0347c03 r9:d7eb3dc8 r8:befe92b0 r7:ede00010 r6:00000000 r5:00000001
 r4:d0320480 r3:bf052058
[<c04fa748>] (__media_entity_setup_link) from [<c04f9bb0>] (media_device_setup_link+0x84/0x90)
 r7:ede00010 r6:ede00010 r5:d039501c r4:d7eb3dc8
[<c04f9b2c>] (media_device_setup_link) from [<c04f9e94>] (media_device_ioctl+0xa4/0x148)
 r6:00000000 r5:d7eb3dc8 r4:c077b014 r3:c04f9b2c
[<c04f9df0>] (media_device_ioctl) from [<c04fa3a0>] (media_ioctl+0x38/0x4c)
 r10:ed5eca68 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:00000003 r5:e82ca500
 r4:c0190304
[<c04fa368>] (media_ioctl) from [<c018f9c0>] (do_vfs_ioctl+0x98/0x9a0)
[<c018f928>] (do_vfs_ioctl) from [<c0190304>] (SyS_ioctl+0x3c/0x60)
 r10:00000000 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:c0347c03 r5:e82ca500
 r4:e82ca500
[<c01902c8>] (SyS_ioctl) from [<c000fd60>] (ret_fast_syscall+0x0/0x1c)
 r8:c000ff04 r7:00000036 r6:000261d0 r5:00000001 r4:0091737c r3:00000001
---[ end trace 4fdd40e5adfc4486 ]---

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598721

On 03/12/2017 10:51 AM, Russell King - ARM Linux wrote:
> I've just looked at my test system's dmesg, and spotted this in the log.
> It's been a while since these popped out of the kernel, so I don't know
> what caused them (other than the obvious, a media-ctl command.)
>
> My script which sets this up only enables links, and then configures the
> formats etc, and doesn't disable them, so I don't see why the power
> count should be going negative.

There's actually nothing preventing userland from disabling a link
multiple times, and imx_media_link_notify() complies, and so
csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
in there is silly, I borrowed this from other MC driver examples,
but it makes no sense to me, I'll remove it and prevent the power
count from going negative.

Steve



>
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1889 at drivers/staging/media/imx/imx-media-csi.c:806 csi_s_power+0x9c/0xa8 [imx_media_csi]
> Modules linked in: caam_jr uvcvideo snd_soc_imx_sgtl5000 snd_soc_fsl_asoc_card snd_soc_imx_spdif imx_media_csi(C) imx6_mipi_csi2(C) snd_soc_imx_audmux snd_soc_sgtl5000 imx219 imx_media_ic(C) imx_media_capture(C) imx_media_vdic(C) caam video_multiplexer imx_sdma coda v4l2_mem2mem videobuf2_v4l2 imx2_wdt imx_vdoa videobuf2_dma_contig videobuf2_core videobuf2_vmalloc videobuf2_memops snd_soc_fsl_ssi
> imx_thermal snd_soc_fsl_spdif imx_pcm_dma imx_media(C) imx_media_common(C) nfsd
> rc_pinnacle_pctv_hd dw_hdmi_ahb_audio dw_hdmi_cec etnaviv
> CPU: 1 PID: 1889 Comm: media-ctl Tainted: G         C      4.11.0-rc1+ #2125
> Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> Backtrace:
> [<c0013ba4>] (dump_backtrace) from [<c0013de4>] (show_stack+0x18/0x1c)
>  r6:600e0013 r5:ffffffff r4:00000000 r3:00000000
> [<c0013dcc>] (show_stack) from [<c033f728>] (dump_stack+0xa4/0xdc)
> [<c033f684>] (dump_stack) from [<c00326cc>] (__warn+0xdc/0x108)
>  r6:bf124014 r5:00000000 r4:00000000 r3:c09ea4a8
> [<c00325f0>] (__warn) from [<c0032720>] (warn_slowpath_null+0x28/0x30)
>  r10:ede00010 r8:ede4a348 r7:d039501c r6:d0395140 r5:00000000 r4:d0395010
> [<c00326f8>] (warn_slowpath_null) from [<bf1229e0>] (csi_s_power+0x9c/0xa8 [imx_media_csi])
> [<bf122944>] (csi_s_power [imx_media_csi]) from [<bf04a930>] (imx_media_set_power+0x3c/0x108 [imx_media_common])
>  r7:d039501c r6:00000000 r5:00000000 r4:0000000c
> [<bf04a8f4>] (imx_media_set_power [imx_media_common]) from [<bf04aa34>] (imx_media_pipeline_set_power+0x38/0x40 [imx_media_common])
>  r10:00000001 r9:00000001 r8:ede4a348 r7:ede00010 r6:ede4a348 r5:d039501c
>  r4:00000001
> [<bf04a9fc>] (imx_media_pipeline_set_power [imx_media_common]) from [<bf052148>] (imx_media_link_notify+0xf0/0x144 [imx_media])
>  r7:ede00010 r6:ed59f900 r5:00000000 r4:d039501c
> [<bf052058>] (imx_media_link_notify [imx_media]) from [<c04fa858>] (__media_entity_setup_link+0x110/0x1d8)
>  r10:c0347c03 r9:d7eb3dc8 r8:befe92b0 r7:ede00010 r6:00000000 r5:00000001
>  r4:ed59f900 r3:bf052058
> [<c04fa748>] (__media_entity_setup_link) from [<c04f9bb0>] (media_device_setup_link+0x84/0x90)
>  r7:ede00010 r6:ede00010 r5:ef3fd810 r4:d7eb3dc8
> [<c04f9b2c>] (media_device_setup_link) from [<c04f9e94>] (media_device_ioctl+0xa4/0x148)
>  r6:00000000 r5:d7eb3dc8 r4:c077b014 r3:c04f9b2c
> [<c04f9df0>] (media_device_ioctl) from [<c04fa3a0>] (media_ioctl+0x38/0x4c)
>  r10:ed5eca68 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:00000003 r5:e82ca280
>  r4:c0190304
> [<c04fa368>] (media_ioctl) from [<c018f9c0>] (do_vfs_ioctl+0x98/0x9a0)
> [<c018f928>] (do_vfs_ioctl) from [<c0190304>] (SyS_ioctl+0x3c/0x60)
>  r10:00000000 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:c0347c03 r5:e82ca280
>  r4:e82ca280
> [<c01902c8>] (SyS_ioctl) from [<c000fd60>] (ret_fast_syscall+0x0/0x1c)
>  r8:c000ff04 r7:00000036 r6:000261d0 r5:00000001 r4:009162e8 r3:00000001
> ---[ end trace 4fdd40e5adfc4485 ]---
> ------------[ cut here ]------------
> WARNING: CPU: 1 PID: 1889 at drivers/staging/media/imx/imx-media-csi.c:806 csi_s_power+0x9c/0xa8 [imx_media_csi]
> Modules linked in: caam_jr uvcvideo snd_soc_imx_sgtl5000 snd_soc_fsl_asoc_card snd_soc_imx_spdif imx_media_csi(C) imx6_mipi_csi2(C) snd_soc_imx_audmux snd_soc_sgtl5000 imx219 imx_media_ic(C) imx_media_capture(C) imx_media_vdic(C) caam video_multiplexer imx_sdma coda v4l2_mem2mem videobuf2_v4l2 imx2_wdt imx_vdoa videobuf2_dma_contig videobuf2_core videobuf2_vmalloc videobuf2_memops snd_soc_fsl_ssi
> imx_thermal snd_soc_fsl_spdif imx_pcm_dma imx_media(C) imx_media_common(C) nfsd
> rc_pinnacle_pctv_hd dw_hdmi_ahb_audio dw_hdmi_cec etnaviv
> CPU: 1 PID: 1889 Comm: media-ctl Tainted: G        WC      4.11.0-rc1+ #2125
> Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> Backtrace:
> [<c0013ba4>] (dump_backtrace) from [<c0013de4>] (show_stack+0x18/0x1c)
>  r6:600e0013 r5:ffffffff r4:00000000 r3:00000000
> [<c0013dcc>] (show_stack) from [<c033f728>] (dump_stack+0xa4/0xdc)
> [<c033f684>] (dump_stack) from [<c00326cc>] (__warn+0xdc/0x108)
>  r6:bf124014 r5:00000000 r4:00000000 r3:c09ea4a8
> [<c00325f0>] (__warn) from [<c0032720>] (warn_slowpath_null+0x28/0x30)
>  r10:ede00010 r8:ede4a348 r7:ee000800 r6:d0395140 r5:00000000 r4:d0395010
> [<c00326f8>] (warn_slowpath_null) from [<bf1229e0>] (csi_s_power+0x9c/0xa8 [imx_media_csi])
> [<bf122944>] (csi_s_power [imx_media_csi]) from [<bf04a930>] (imx_media_set_power+0x3c/0x108 [imx_media_common])
>  r7:ee000800 r6:00000000 r5:00000000 r4:0000000c
> [<bf04a8f4>] (imx_media_set_power [imx_media_common]) from [<bf04aa34>] (imx_media_pipeline_set_power+0x38/0x40 [imx_media_common])
>  r10:00000001 r9:00000001 r8:ede4a348 r7:ede00010 r6:ede4a348 r5:ee000800
>  r4:00000001
> [<bf04a9fc>] (imx_media_pipeline_set_power [imx_media_common]) from [<bf052148>] (imx_media_link_notify+0xf0/0x144 [imx_media])
>  r7:ede00010 r6:d0320480 r5:ee000800 r4:ee000800
> [<bf052058>] (imx_media_link_notify [imx_media]) from [<c04fa858>] (__media_entity_setup_link+0x110/0x1d8)
>  r10:c0347c03 r9:d7eb3dc8 r8:befe92b0 r7:ede00010 r6:00000000 r5:00000001
>  r4:d0320480 r3:bf052058
> [<c04fa748>] (__media_entity_setup_link) from [<c04f9bb0>] (media_device_setup_link+0x84/0x90)
>  r7:ede00010 r6:ede00010 r5:d039501c r4:d7eb3dc8
> [<c04f9b2c>] (media_device_setup_link) from [<c04f9e94>] (media_device_ioctl+0xa4/0x148)
>  r6:00000000 r5:d7eb3dc8 r4:c077b014 r3:c04f9b2c
> [<c04f9df0>] (media_device_ioctl) from [<c04fa3a0>] (media_ioctl+0x38/0x4c)
>  r10:ed5eca68 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:00000003 r5:e82ca500
>  r4:c0190304
> [<c04fa368>] (media_ioctl) from [<c018f9c0>] (do_vfs_ioctl+0x98/0x9a0)
> [<c018f928>] (do_vfs_ioctl) from [<c0190304>] (SyS_ioctl+0x3c/0x60)
>  r10:00000000 r9:d7eb2000 r8:befe92b0 r7:00000003 r6:c0347c03 r5:e82ca500
>  r4:e82ca500
> [<c01902c8>] (SyS_ioctl) from [<c000fd60>] (ret_fast_syscall+0x0/0x1c)
>  r8:c000ff04 r7:00000036 r6:000261d0 r5:00000001 r4:0091737c r3:00000001
> ---[ end trace 4fdd40e5adfc4486 ]---
>

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

#1598724

On Sun, Mar 12, 2017 at 12:21:45PM -0700, Steve Longerbeam wrote:
> There's actually nothing preventing userland from disabling a link
> multiple times, and imx_media_link_notify() complies, and so
> csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
> in there is silly, I borrowed this from other MC driver examples,
> but it makes no sense to me, I'll remove it and prevent the power
> count from going negative.

Hmm.  So what happens if one of the CSI's links is enabled, and we
disable a different link from the CSI several times?  Doesn't that
mean the power count will go to zero despite there being an enabled
link?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

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

#1598725

On 03/12/2017 12:29 PM, Russell King - ARM Linux wrote:
> On Sun, Mar 12, 2017 at 12:21:45PM -0700, Steve Longerbeam wrote:
>> There's actually nothing preventing userland from disabling a link
>> multiple times, and imx_media_link_notify() complies, and so
>> csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
>> in there is silly, I borrowed this from other MC driver examples,
>> but it makes no sense to me, I'll remove it and prevent the power
>> count from going negative.
>
> Hmm.  So what happens if one of the CSI's links is enabled, and we
> disable a different link from the CSI several times?  Doesn't that
> mean the power count will go to zero despite there being an enabled
> link?

Yes, the CSI will be powered off even if it still has an enabled link.
But one of its other links has been disabled, meaning the pipeline as
a whole is disabled. So I think it makes sense to power down the CSI,
the pipeline isn't usable at that point.

And remember that the CSI does not allow both output pads to be enabled
at the same time. If that were so then indeed there would be a problem,
because it would mean there is another active pipeline that requires the
CSI being powered on, but that's not the case.

I think this is consistent with the other entities as well, but I will
double check.

Steve

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

#1598739

On 03/12/2017 12:44 PM, Steve Longerbeam wrote:
>
>
> On 03/12/2017 12:29 PM, Russell King - ARM Linux wrote:
>> On Sun, Mar 12, 2017 at 12:21:45PM -0700, Steve Longerbeam wrote:
>>> There's actually nothing preventing userland from disabling a link
>>> multiple times, and imx_media_link_notify() complies, and so
>>> csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
>>> in there is silly, I borrowed this from other MC driver examples,
>>> but it makes no sense to me, I'll remove it and prevent the power
>>> count from going negative.
>>
>> Hmm.  So what happens if one of the CSI's links is enabled, and we
>> disable a different link from the CSI several times?  Doesn't that
>> mean the power count will go to zero despite there being an enabled
>> link?
>
> Yes, the CSI will be powered off even if it still has an enabled link.
> But one of its other links has been disabled, meaning the pipeline as
> a whole is disabled. So I think it makes sense to power down the CSI,
> the pipeline isn't usable at that point.
>
> And remember that the CSI does not allow both output pads to be enabled
> at the same time. If that were so then indeed there would be a problem,
> because it would mean there is another active pipeline that requires the
> CSI being powered on, but that's not the case.
>
> I think this is consistent with the other entities as well, but I will
> double check.


At first I thought this could be a problem for one entity, the csi-2
receiver.

It can enable all four of its output pads at once (if the input stream
contains all 4 virtual channels, the csi-2 receiver must support
demuxing all of them onto all 4 of its output pads).

But after more review, this should not be an issue. If a csi-2 sink
(a CSI or a CSI mux) link is disabled, the csi-2 receiver is no longer
reachable from that sink, so attempts to disable the csi-2 via that
path again is not possible. The other potential problem is disabling
from the csi-2's own sink pad, but in that case the csi-2 no longer
has a source, so again it makes sense to power off the csi-2 even
if it has enabled output pads.


Steve

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

#1598743

On 03/12/2017 01:16 PM, Steve Longerbeam wrote:
>
>
> On 03/12/2017 12:44 PM, Steve Longerbeam wrote:
>>
>>
>> On 03/12/2017 12:29 PM, Russell King - ARM Linux wrote:
>>> On Sun, Mar 12, 2017 at 12:21:45PM -0700, Steve Longerbeam wrote:
>>>> There's actually nothing preventing userland from disabling a link
>>>> multiple times, and imx_media_link_notify() complies, and so
>>>> csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
>>>> in there is silly, I borrowed this from other MC driver examples,
>>>> but it makes no sense to me, I'll remove it and prevent the power
>>>> count from going negative.
>>>
>>> Hmm.  So what happens if one of the CSI's links is enabled, and we
>>> disable a different link from the CSI several times?  Doesn't that
>>> mean the power count will go to zero despite there being an enabled
>>> link?
>>
>> Yes, the CSI will be powered off even if it still has an enabled link.
>> But one of its other links has been disabled, meaning the pipeline as
>> a whole is disabled. So I think it makes sense to power down the CSI,
>> the pipeline isn't usable at that point.
>>
>> And remember that the CSI does not allow both output pads to be enabled
>> at the same time. If that were so then indeed there would be a problem,
>> because it would mean there is another active pipeline that requires the
>> CSI being powered on, but that's not the case.
>>
>> I think this is consistent with the other entities as well, but I will
>> double check.
>
>
> At first I thought this could be a problem for one entity, the csi-2
> receiver.
>
> It can enable all four of its output pads at once (if the input stream
> contains all 4 virtual channels, the csi-2 receiver must support
> demuxing all of them onto all 4 of its output pads).
>
> But after more review, this should not be an issue. If a csi-2 sink
> (a CSI or a CSI mux) link is disabled, the csi-2 receiver is no longer
> reachable from that sink, so attempts to disable the csi-2 via that
> path again is not possible. The other potential problem is disabling
> from the csi-2's own sink pad, but in that case the csi-2 no longer
> has a source, so again it makes sense to power off the csi-2 even
> if it has enabled output pads.
>

But hold on, if my logic is correct, then why did the CSI power-off
get reached in your case, multiple times? Yes I think there is a bug,
link_notify() is not checking if the link has already been disabled.
I will fix this. But I'm surprised media core's link_notify handling
doesn't do this.

Steve

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

#1598744

On 03/12/2017 01:36 PM, Steve Longerbeam wrote:
>
>
> On 03/12/2017 01:16 PM, Steve Longerbeam wrote:
>>
>>
>> On 03/12/2017 12:44 PM, Steve Longerbeam wrote:
>>>
>>>
>>> On 03/12/2017 12:29 PM, Russell King - ARM Linux wrote:
>>>> On Sun, Mar 12, 2017 at 12:21:45PM -0700, Steve Longerbeam wrote:
>>>>> There's actually nothing preventing userland from disabling a link
>>>>> multiple times, and imx_media_link_notify() complies, and so
>>>>> csi_s_power(OFF) gets called multiple times, and so that WARN_ON()
>>>>> in there is silly, I borrowed this from other MC driver examples,
>>>>> but it makes no sense to me, I'll remove it and prevent the power
>>>>> count from going negative.
>>>>
>>>> Hmm.  So what happens if one of the CSI's links is enabled, and we
>>>> disable a different link from the CSI several times?  Doesn't that
>>>> mean the power count will go to zero despite there being an enabled
>>>> link?
>>>
>>> Yes, the CSI will be powered off even if it still has an enabled link.
>>> But one of its other links has been disabled, meaning the pipeline as
>>> a whole is disabled. So I think it makes sense to power down the CSI,
>>> the pipeline isn't usable at that point.
>>>
>>> And remember that the CSI does not allow both output pads to be enabled
>>> at the same time. If that were so then indeed there would be a problem,
>>> because it would mean there is another active pipeline that requires the
>>> CSI being powered on, but that's not the case.
>>>
>>> I think this is consistent with the other entities as well, but I will
>>> double check.
>>
>>
>> At first I thought this could be a problem for one entity, the csi-2
>> receiver.
>>
>> It can enable all four of its output pads at once (if the input stream
>> contains all 4 virtual channels, the csi-2 receiver must support
>> demuxing all of them onto all 4 of its output pads).
>>
>> But after more review, this should not be an issue. If a csi-2 sink
>> (a CSI or a CSI mux) link is disabled, the csi-2 receiver is no longer
>> reachable from that sink, so attempts to disable the csi-2 via that
>> path again is not possible. The other potential problem is disabling
>> from the csi-2's own sink pad, but in that case the csi-2 no longer
>> has a source, so again it makes sense to power off the csi-2 even
>> if it has enabled output pads.
>>
>
> But hold on, if my logic is correct, then why did the CSI power-off
> get reached in your case, multiple times? Yes I think there is a bug,
> link_notify() is not checking if the link has already been disabled.
> I will fix this. But I'm surprised media core's link_notify handling
> doesn't do this.

but it does:

int __media_entity_setup_link(struct media_link *link, u32 flags)
{
...
     if (link->flags == flags)
         return 0;
...
}

What the heck. Anyway, I'll track this down.

Steve

>
> Steve
>

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

Page 4 of 5 — ← Prev page 1 2 3 [4] 5  Next page →

Back to top | Article view | linux.kernel

csiph-web