Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates

From	Steve Longerbeam <slongerbeam@gmail.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates
Date	2017-02-21 01:20 +0100
Message-ID	<td6lr-3jL-3@gated-at.bofh.it> (permalink)
References	<tbjZv-8cO-3@gated-at.bofh.it> <tbjZx-8cO-53@gated-at.bofh.it> <td4jD-1Zp-7@gated-at.bofh.it> <td6lr-3jL-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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

On 02/20/2017 04:13 PM, Russell King - ARM Linux wrote:
> On Tue, Feb 21, 2017 at 12:04:10AM +0200, Sakari Ailus wrote:
>> On Wed, Feb 15, 2017 at 06:19:31PM -0800, Steve Longerbeam wrote:
>>> From: Russell King <rmk+kernel@armlinux.org.uk>
>>>
>>> Setting and getting frame rates is part of the negotiation mechanism
>>> between subdevs.  The lack of support means that a frame rate at the
>>> sensor can't be negotiated through the subdev path.
>>
>> Just wondering --- what do you need this for?
>
> The v4l2 documentation contradicts the media-ctl implementation.
>
> While v4l2 documentation says:
>
>   These ioctls are used to get and set the frame interval at specific
>   subdev pads in the image pipeline. The frame interval only makes sense
>   for sub-devices that can control the frame period on their own. This
>   includes, for instance, image sensors and TV tuners. Sub-devices that
>   don't support frame intervals must not implement these ioctls.
>
> However, when trying to configure the pipeline using media-ctl, eg:
>
> media-ctl -d /dev/media1 --set-v4l2 '"imx219 pixel 0-0010":0[crop:(0,0)/3264x2464]'
> media-ctl -d /dev/media1 --set-v4l2 '"imx219 0-0010":1[fmt:SRGGB10/3264x2464@1/30]'
> media-ctl -d /dev/media1 --set-v4l2 '"imx219 0-0010":0[fmt:SRGGB8/816x616@1/30]'
> media-ctl -d /dev/media1 --set-v4l2 '"imx6-mipi-csi2":1[fmt:SRGGB8/816x616@1/30]'
> Unable to setup formats: Inappropriate ioctl for device (25)
> media-ctl -d /dev/media1 --set-v4l2 '"ipu1_csi0_mux":2[fmt:SRGGB8/816x616@1/30]'
> media-ctl -d /dev/media1 --set-v4l2 '"ipu1_csi0":2[fmt:SRGGB8/816x616@1/30]'
>
> The problem there is that the format setting for the csi2 does not get
> propagated forward:
>
> $ strace media-ctl -d /dev/media1 --set-v4l2 '"imx6-mipi-csi2":1[fmt:SRGGB8/816x616@1/30]'
> ...
> open("/dev/v4l-subdev16", O_RDWR)       = 3
> ioctl(3, VIDIOC_SUBDEV_S_FMT, 0xbec16244) = 0
> ioctl(3, VIDIOC_SUBDEV_S_FRAME_INTERVAL, 0xbec162a4) = -1 ENOTTY (Inappropriate
> ioctl for device)
> fstat64(1, {st_mode=S_IFCHR|0600, st_rdev=makedev(136, 2), ...}) = 0
> write(1, "Unable to setup formats: Inappro"..., 61) = 61
> Unable to setup formats: Inappropriate ioctl for device (25)
> close(3)                                = 0
> exit_group(1)                           = ?
> +++ exited with 1 +++
>
> because media-ctl exits as soon as it encouters the error while trying
> to set the frame rate.
>
> This makes implementing setup of the media pipeline in shell scripts
> unnecessarily difficult - as you need to then know whether an entity
> is likely not to support the VIDIOC_SUBDEV_S_FRAME_INTERVAL call,
> and either avoid specifying a frame rate:
>
> $ strace media-ctl -d /dev/media1 --set-v4l2 '"imx6-mipi-csi2":1[fmt:SRGGB8/816x616]'
> ...
> open("/dev/v4l-subdev16", O_RDWR)       = 3
> ioctl(3, VIDIOC_SUBDEV_S_FMT, 0xbeb1a254) = 0
> open("/dev/v4l-subdev0", O_RDWR)        = 4
> ioctl(4, VIDIOC_SUBDEV_S_FMT, 0xbeb1a254) = 0
> close(4)                                = 0
> close(3)                                = 0
> exit_group(0)                           = ?
> +++ exited with 0 +++
>
> or manually setting the format on the sink.
>
> Allowing the S_FRAME_INTERVAL call seems to me to be more in keeping
> with the negotiation mechanism that is implemented in subdevs, and
> IMHO should be implemented inside the kernel as a pad operation along
> with the format negotiation, especially so as frame skipping is
> defined as scaling, in just the same way as the frame size is also
> scaling:
>
>        -  ``MEDIA_ENT_F_PROC_VIDEO_SCALER``
>
>        -  Video scaler. An entity capable of video scaling must have
>           at least one sink pad and one source pad, and scale the
>           video frame(s) received on its sink pad(s) to a different
>           resolution output on its source pad(s). The range of
>           supported scaling ratios is entity-specific and can differ
>           between the horizontal and vertical directions (in particular
>           scaling can be supported in one direction only). Binning and
>           skipping are considered as scaling.
>
> Although, this is vague, as it doesn't define what it means by "skipping",
> whether that's skipping pixels (iow, sub-sampling) or whether that's
> frame skipping.
>
> Then there's the issue where, if you have this setup:
>
>  camera --> csi2 receiver --> csi --> capture
>
> and the "csi" subdev can skip frames, you need to know (a) at the CSI
> sink pad what the frame rate is of the source (b) what the desired
> source pad frame rate is, so you can configure the frame skipping.
> So, does the csi subdev have to walk back through the media graph
> looking for the frame rate?  Does the capture device have to walk back
> through the media graph looking for some subdev to tell it what the
> frame rate is - the capture device certainly can't go straight to the
> sensor to get an answer to that question, because that bypasses the
> effect of the CSI frame skipping (which will lower the frame rate.)
>
> IMHO, frame rate is just another format property, just like the
> resolution and data format itself, and v4l2 should be treating it no
> differently.
>

I agree, frame rate, if indicated/specified by both sides of a link,
should match. So maybe this should be part of v4l2 link validation.

This might be a good time to propose the following patch.

Steve

Thread

[PATCH v4 00/36] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 05/36] ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 17/36] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 17/36] media: Add userspace header file for i.MX Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-16 12:40 +0100
      Re: [PATCH v4 17/36] media: Add userspace header file for i.MX Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 01:00 +0100
  [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 11:30 +0100
      Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 19:10 +0100
    Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 12:00 +0100
      Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-17 12:20 +0100
        Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 12:40 +0100
          Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 00:50 +0100
            Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 00:50 +0100
        Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 01:10 +0100
          Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 01:20 +0100
      Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 15:20 +0100
        Re: [PATCH v4 23/36] media: imx: Add MIPI CSI-2 Receiver subdev  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-17 19:30 +0100
  [PATCH v4 28/36] media: imx: csi: fix crop rectangle changes in set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 28/36] media: imx: csi: fix crop rectangle changes in  set_fmt Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 12:10 +0100
      Re: [PATCH v4 28/36] media: imx: csi: fix crop rectangle changes in  set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 19:20 +0100
  [PATCH v4 35/36] media: imx: csi: fix crop rectangle reset in sink set_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 27/36] media: imx: csi: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 33/36] media: imx: redo pixel format enumeration and negotiation Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 33/36] media: imx: redo pixel format enumeration and  negotiation Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-16 12:40 +0100
      Re: [PATCH v4 33/36] media: imx: redo pixel format enumeration and  negotiation Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-23 01:00 +0100
        Re: [PATCH v4 33/36] media: imx: redo pixel format enumeration and  negotiation Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-23 10:20 +0100
          Re: [PATCH v4 33/36] media: imx: redo pixel format enumeration and  negotiation Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-24 02:50 +0100
  [PATCH v4 26/36] media: imx: add support for bayer formats Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 07/36] ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 07/36] ARM: dts: imx6-sabresd: add OV5642 and OV5640  camera sensors Fabio Estevam <festevam@gmail.com> - 2017-02-17 02:00 +0100
      Re: [PATCH v4 07/36] ARM: dts: imx6-sabresd: add OV5642 and OV5640  camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-17 02:00 +0100
  [PATCH v4 10/36] ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 06/36] ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-18 02:20 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-18 10:30 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-18 18:30 +0100
          Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-18 19:20 +0100
    Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-02-18 02:20 +0100
    Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-20 23:10 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-21 00:00 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-21 00:50 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-21 13:20 +0100
          Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-21 23:30 +0100
            Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-22 00:40 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-21 01:20 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-21 10:00 +0100
      Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-21 01:20 +0100
        Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-21 13:40 +0100
          Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-21 14:30 +0100
            Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-21 16:40 +0100
              Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-21 17:10 +0100
                Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and  getting of frame rates Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-21 17:20 +0100
  [PATCH v4 13/36] [media] v4l2: add a frame timeout event Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 31/36] media: imx: csi: add __csi_get_fmt Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 12/36] add mux and video interface bridge entity functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 12/36] add mux and video interface bridge entity  functions Pavel Machek <pavel@ucw.cz> - 2017-02-19 22:40 +0100
      Re: [PATCH v4 12/36] add mux and video interface bridge entity  functions Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-22 18:30 +0100
  [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit  controls from a pipeline Pavel Machek <pavel@ucw.cz> - 2017-02-19 23:00 +0100
  [PATCH v4 15/36] platform: add video-multiplexer subdevice driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver Pavel Machek <pavel@ucw.cz> - 2017-02-19 23:20 +0100
      Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice  driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-21 10:20 +0100
        Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver Pavel Machek <pavel@ucw.cz> - 2017-02-24 21:10 +0100
    Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver Rob Herring <robh@kernel.org> - 2017-02-27 15:50 +0100
      Re: [PATCH v4 15/36] platform: add video-multiplexer subdevice driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-01 01:30 +0100
  [PATCH v4 21/36] media: imx: Add VDIC subdev driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 32/36] media: imx: csi/fim: add support for frame intervals Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
    Re: [PATCH v4 32/36] media: imx: csi/fim: add support for frame  intervals Steve Longerbeam <steve_longerbeam@mentor.com> - 2017-02-16 03:40 +0100
  [PATCH v4 09/36] ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:30 +0100
  [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX media driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:40 +0100
    Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX  media driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-16 13:00 +0100
      Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX media  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 20:30 +0100
    Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX  media driver Rob Herring <robh@kernel.org> - 2017-02-27 15:50 +0100
      Re: [PATCH v4 01/36] [media] dt-bindings: Add bindings for i.MX media  driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-01 01:40 +0100
  [PATCH v4 04/36] ARM: dts: imx6qdl: add capture-subsystem device Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 03:40 +0100
  Re: [PATCH v4 18/36] media: Add i.MX media core driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 11:30 +0100
    Re: [PATCH v4 18/36] media: Add i.MX media core driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 19:00 +0100
  Re: [PATCH v4 00/36] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 12:40 +0100
    Re: [PATCH v4 00/36] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 19:40 +0100
  Re: [PATCH v4 18/36] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-16 14:10 +0100
    Re: [PATCH v4 18/36] media: Add i.MX media core driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 14:50 +0100
    Re: [PATCH v4 18/36] media: Add i.MX media core driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-17 02:40 +0100
      Re: [PATCH v4 18/36] media: Add i.MX media core driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 09:40 +0100
  Re: [PATCH v4 00/36] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-16 23:30 +0100
    Re: [PATCH v4 00/36] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-17 00:10 +0100
      Re: [PATCH v4 00/36] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 11:50 +0100
        Re: [PATCH v4 00/36] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-17 12:00 +0100
          Re: [PATCH v4 00/36] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 12:30 +0100
      Re: [PATCH v4 00/36] i.MX Media Driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-02-18 18:40 +0100
    Re: [PATCH v4 00/36] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 12:50 +0100
      Re: [PATCH v4 00/36] i.MX Media Driver Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-17 13:30 +0100
        Re: [PATCH v4 00/36] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-17 13:40 +0100
        Re: [PATCH v4 00/36] i.MX Media Driver Philipp Zabel <p.zabel@pengutronix.de> - 2017-02-17 16:10 +0100
          Re: [PATCH v4 00/36] i.MX Media Driver Sakari Ailus <sakari.ailus@iki.fi> - 2017-02-18 13:10 +0100
  Re: [PATCH v4 00/36] i.MX Media Driver Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-02-16 23:30 +0100
  Re: [PATCH v4 24/36] [media] add Omnivision OV5640 sensor driver Rob Herring <robh@kernel.org> - 2017-02-27 15:50 +0100
    Re: [PATCH v4 24/36] [media] add Omnivision OV5640 sensor driver Steve Longerbeam <slongerbeam@gmail.com> - 2017-03-01 01:50 +0100

csiph-web