Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1582252 > unrolled thread
| Started by | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| First post | 2017-02-16 03:30 +0100 |
| Last post | 2017-03-01 01:50 +0100 |
| Articles | 20 on this page of 98 — 8 participants |
Back to article view | Back to linux.kernel
[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
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-21 00:00 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <td562-2h7-7@gated-at.bofh.it> |
| In reply to | #1584930 |
On 02/20/2017 02:04 PM, Sakari Ailus wrote: > Hi Steve, > > 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? Hi Sakari, i.MX does need the ability to negotiate the frame rates in the pipelines. The CSI has the ability to skip frames at the output, which is something Philipp added to the CSI subdev. That affects frame interval at the CSI output. But as Russell pointed out, the lack of [gs]_frame_interval op causes media-ctl to fail: media-ctl -v -d /dev/media1 --set-v4l2 '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' Opening media device /dev/media1 Enumerating entities Found 29 entities Enumerating pads and links Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 Format set: SGBRG8 512x512 Setting up frame interval 1/30 on entity imx6-mipi-csi2 Unable to set frame interval: Inappropriate ioctl for device (-25)Unable to setup formats: Inappropriate ioctl for device (25) So i.MX needs to implement this op in every subdev in the pipeline, otherwise it's not possible to configure the pipeline with media-ctl. Steve
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-21 00:50 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <td5Sp-2NQ-3@gated-at.bofh.it> |
| In reply to | #1584953 |
On 02/20/2017 02:56 PM, Steve Longerbeam wrote: > > > On 02/20/2017 02:04 PM, Sakari Ailus wrote: >> Hi Steve, >> >> 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? > > > Hi Sakari, > > i.MX does need the ability to negotiate the frame rates in the > pipelines. The CSI has the ability to skip frames at the output, > which is something Philipp added to the CSI subdev. That affects > frame interval at the CSI output. > > But as Russell pointed out, the lack of [gs]_frame_interval op > causes media-ctl to fail: > > media-ctl -v -d /dev/media1 --set-v4l2 > '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' > > Opening media device /dev/media1 > Enumerating entities > Found 29 entities > Enumerating pads and links > Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 > Format set: SGBRG8 512x512 > Setting up frame interval 1/30 on entity imx6-mipi-csi2 > Unable to set frame interval: Inappropriate ioctl for device (-25)Unable > to setup formats: Inappropriate ioctl for device (25) > > > So i.MX needs to implement this op in every subdev in the > pipeline, otherwise it's not possible to configure the > pipeline with media-ctl. > Hi Russell, But Sakari brings up a good point. The mipi csi-2 receiver doesn't have any control over frame rate, so why do you even need to give it this information via media-ctl? Steve
[toc] | [prev] | [next] | [standalone]
| From | Sakari Ailus <sakari.ailus@iki.fi> |
|---|---|
| Date | 2017-02-21 13:20 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdhAd-2rg-1@gated-at.bofh.it> |
| In reply to | #1584953 |
Hi Steve, On Mon, Feb 20, 2017 at 02:56:15PM -0800, Steve Longerbeam wrote: > > > On 02/20/2017 02:04 PM, Sakari Ailus wrote: > >Hi Steve, > > > >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? > > > Hi Sakari, > > i.MX does need the ability to negotiate the frame rates in the > pipelines. The CSI has the ability to skip frames at the output, > which is something Philipp added to the CSI subdev. That affects > frame interval at the CSI output. > > But as Russell pointed out, the lack of [gs]_frame_interval op > causes media-ctl to fail: > > media-ctl -v -d /dev/media1 --set-v4l2 > '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' > > Opening media device /dev/media1 > Enumerating entities > Found 29 entities > Enumerating pads and links > Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 > Format set: SGBRG8 512x512 > Setting up frame interval 1/30 on entity imx6-mipi-csi2 > Unable to set frame interval: Inappropriate ioctl for device (-25)Unable to > setup formats: Inappropriate ioctl for device (25) > > > So i.MX needs to implement this op in every subdev in the > pipeline, otherwise it's not possible to configure the > pipeline with media-ctl. The frame rate is only set on the sub-device which you explicitly set it. I.e. setting the frame rate fails if it's not supported on a pad. Philipp recently posted patches that add frame rate propagation to media-ctl. Frame rate is typically settable (and gettable) only on sensor sub-device's source pad, which means it normally would not be propagated by the kernel but with Philipp's patches, on the sink pad of the bus receiver. Receivers don't have a way to control it nor they implement the IOCTLs, so that would indeed result in an error. -- Kind regards, Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-21 23:30 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdr6y-on-23@gated-at.bofh.it> |
| In reply to | #1585261 |
On 02/21/2017 04:15 AM, Sakari Ailus wrote: > Hi Steve, > > On Mon, Feb 20, 2017 at 02:56:15PM -0800, Steve Longerbeam wrote: >> >> >> On 02/20/2017 02:04 PM, Sakari Ailus wrote: >>> Hi Steve, >>> >>> 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? >> >> >> Hi Sakari, >> >> i.MX does need the ability to negotiate the frame rates in the >> pipelines. The CSI has the ability to skip frames at the output, >> which is something Philipp added to the CSI subdev. That affects >> frame interval at the CSI output. >> >> But as Russell pointed out, the lack of [gs]_frame_interval op >> causes media-ctl to fail: >> >> media-ctl -v -d /dev/media1 --set-v4l2 >> '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' >> >> Opening media device /dev/media1 >> Enumerating entities >> Found 29 entities >> Enumerating pads and links >> Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 >> Format set: SGBRG8 512x512 >> Setting up frame interval 1/30 on entity imx6-mipi-csi2 >> Unable to set frame interval: Inappropriate ioctl for device (-25)Unable to >> setup formats: Inappropriate ioctl for device (25) >> >> >> So i.MX needs to implement this op in every subdev in the >> pipeline, otherwise it's not possible to configure the >> pipeline with media-ctl. > > The frame rate is only set on the sub-device which you explicitly set it. > I.e. setting the frame rate fails if it's not supported on a pad. > > Philipp recently posted patches that add frame rate propagation to > media-ctl. > > Frame rate is typically settable (and gettable) only on sensor sub-device's > source pad, which means it normally would not be propagated by the kernel > but with Philipp's patches, on the sink pad of the bus receiver. Receivers > don't have a way to control it nor they implement the IOCTLs, so that would > indeed result in an error. > Frame rate is really an essential piece of information. The spatial dimensions and data type provided by set_fmt are really only half the equation, the other is temporal, i.e. the data rate. It's true that subdevices have no control over the frame rate at their sink pads, but the same argument applies to set_fmt. Even if it has no control over the data format it receives, it still needs that information in order to determine the correct format at the source. The same argument applies to frame rate. So in my opinion, the behavior of [gs]_frame_interval should be, if a subdevice is capable of modifying the frame rate, then it should implement [gs]_frame_interval at _all_ of its pads, similar to set_fmt. And frame rate should really be part of link validation the same as set_fmt is. Steve
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-22 00:40 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdsch-18h-5@gated-at.bofh.it> |
| In reply to | #1585784 |
On 02/21/2017 02:21 PM, Steve Longerbeam wrote: > > > On 02/21/2017 04:15 AM, Sakari Ailus wrote: >> Hi Steve, >> >> On Mon, Feb 20, 2017 at 02:56:15PM -0800, Steve Longerbeam wrote: >>> >>> >>> On 02/20/2017 02:04 PM, Sakari Ailus wrote: >>>> Hi Steve, >>>> >>>> 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? >>> >>> >>> Hi Sakari, >>> >>> i.MX does need the ability to negotiate the frame rates in the >>> pipelines. The CSI has the ability to skip frames at the output, >>> which is something Philipp added to the CSI subdev. That affects >>> frame interval at the CSI output. >>> >>> But as Russell pointed out, the lack of [gs]_frame_interval op >>> causes media-ctl to fail: >>> >>> media-ctl -v -d /dev/media1 --set-v4l2 >>> '"imx6-mipi-csi2":1[fmt:SGBRG8/512x512@1/30]' >>> >>> Opening media device /dev/media1 >>> Enumerating entities >>> Found 29 entities >>> Enumerating pads and links >>> Setting up format SGBRG8 512x512 on pad imx6-mipi-csi2/1 >>> Format set: SGBRG8 512x512 >>> Setting up frame interval 1/30 on entity imx6-mipi-csi2 >>> Unable to set frame interval: Inappropriate ioctl for device >>> (-25)Unable to >>> setup formats: Inappropriate ioctl for device (25) >>> >>> >>> So i.MX needs to implement this op in every subdev in the >>> pipeline, otherwise it's not possible to configure the >>> pipeline with media-ctl. >> >> The frame rate is only set on the sub-device which you explicitly set it. >> I.e. setting the frame rate fails if it's not supported on a pad. >> >> Philipp recently posted patches that add frame rate propagation to >> media-ctl. >> >> Frame rate is typically settable (and gettable) only on sensor >> sub-device's >> source pad, which means it normally would not be propagated by the >> kernel >> but with Philipp's patches, on the sink pad of the bus receiver. >> Receivers >> don't have a way to control it nor they implement the IOCTLs, so that >> would >> indeed result in an error. >> > > Frame rate is really an essential piece of information. The spatial > dimensions and data type provided by set_fmt are really only half the > equation, the other is temporal, i.e. the data rate. > > It's true that subdevices have no control over the frame rate at their > sink pads, but the same argument applies to set_fmt. Even if it has > no control over the data format it receives, it still needs that > information in order to determine the correct format at the source. > The same argument applies to frame rate. > > So in my opinion, the behavior of [gs]_frame_interval should be, if a > subdevice is capable of modifying the frame rate, then it should > implement [gs]_frame_interval at _all_ of its pads, similar to set_fmt. > And frame rate should really be part of link validation the same as > set_fmt is. > Actually, if frame rate were added to link validation then [gs]_frame_interval would have to be mandatory, even if the subdev has no control over frame rate, again this is like set_fmt. Otherwise, if a subdev has not implemented [gs]_frame_interval, then frame rate validation across the whole pipeline is broken. Because, if we have A -> B -> C and B has not implemented [gs]_frame_interval, and C is expecting 30 fps, then pipeline validation would succeed even though A is outputting 60 fps. Steve
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-21 01:20 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <td6lr-3jL-3@gated-at.bofh.it> |
| In reply to | #1584930 |
[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
[toc] | [prev] | [next] | [standalone]
| From | Philipp Zabel <p.zabel@pengutronix.de> |
|---|---|
| Date | 2017-02-21 10:00 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdesF-af-5@gated-at.bofh.it> |
| In reply to | #1584971 |
On Mon, 2017-02-20 at 16:18 -0800, Steve Longerbeam wrote:
>
> 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.
I'd interpret this as meaning pixel skipping, not 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.
I agree with Steve and Russell. I don't see why the (nominal) frame
interval should be handled differently than resolution, data format, and
colorspace information. I think it should just be propagated in the same
way, and there is no reason to have two connected pads set to a
different interval. That would make implementing the g/s_frame_interval
subdev calls mandatory.
regards
Philipp
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2017-02-21 01:20 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <td6lr-3jL-5@gated-at.bofh.it> |
| In reply to | #1584930 |
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.
In any case, the documentation vs media-ctl create something of a very
obscure situation, one that probably needs solving one way or another.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Sakari Ailus <sakari.ailus@iki.fi> |
|---|---|
| Date | 2017-02-21 13:40 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdhTA-2y5-11@gated-at.bofh.it> |
| In reply to | #1584972 |
Hi Russell,
On Tue, Feb 21, 2017 at 12:13:32AM +0000, 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:
The CSI-2 receivers typically do not implement frame interval IOCTLs as they
do not control the frame interval. Some sensors or TV tuners typically do,
so they implement these IOCTLs.
There are alternative ways to specify the frame rate.
>
> $ 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:
You should remove the frame interval setting from sub-devices that do not
support it.
>
> $ 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:
The origins of the S_FRAME_INTERVAL IOCTL for sub-devices are the S_PARM
IOCTL for video nodes. It is used to control the frame rate for more simple
devices that do not expose the Media controller interface. The similar
S_FRAME_INTERVAL was added for sub-devices as well, and it has been so far
used to control the frame interval for sensors (and G_FRAME_INTERVAL to
obtain the frame interval for TV tuners, for instance).
The pad argument was added there but media-ctl only supported setting the
frame interval on pad 0, which, coincidentally, worked well for sensor
devices.
The link validation is primarily done in order to ensure the validity of the
hardware configuration: streaming may not be started if the hardware
configuration is not valid.
Also, frame interval is not a static property during streaming: it may be
changed without the knowledge of the other sub-device drivers downstream. It
neither is a property of hardware receiving or processing images: if there
are limitations in processing pixels, then they in practice are related to
pixel rates or image sizes (i.e. not frame rates).
>
> - ``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.
Skipping in the context is used to refer to sub-sampling. The term is often
used in conjunction of sensors. The documentation could certainly be
clarified here.
>
> 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.
>
> In any case, the documentation vs media-ctl create something of a very
> obscure situation, one that probably needs solving one way or another.
Before going to solutions I need to ask: what do you want to achieve?
--
Regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2017-02-21 14:30 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdiFY-37b-11@gated-at.bofh.it> |
| In reply to | #1585271 |
On Tue, Feb 21, 2017 at 02:37:57PM +0200, Sakari Ailus wrote:
> Hi Russell,
>
> On Tue, Feb 21, 2017 at 12:13:32AM +0000, 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:
>
> The CSI-2 receivers typically do not implement frame interval IOCTLs as they
> do not control the frame interval. Some sensors or TV tuners typically do,
> so they implement these IOCTLs.
No, TV tuners do not. The frame rate for a TV tuner is set by the
broadcaster, not by the tuner. The tuner can't change that frame rate.
The tuner may opt to "skip" fields or frames. That's no different from
what the CSI block in my example below is capable of doing.
Treating a tuner differently from the CSI block is inconsistent and
completely wrong.
> There are alternative ways to specify the frame rate.
Empty statements (or hand-waving type statements) I'm afraid don't
contribute to the discussion, because they mean nothing to me. Please
give an example, or flesh out what you mean.
> > $ 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:
>
> You should remove the frame interval setting from sub-devices that do not
> support it.
That means we end up with horribly complex scripts. This "solution" does
not scale. Therefore, it is not a "solution".
It's fine if you want to write a script to setup the media pipeline using
media-ctl, listing _each_ media-ctl command individually, with arguments
specific to each step, but as I've already said, that does not scale.
I don't want to end up writing separate scripts to configure the pipeline
for different parameters or setups. I don't want to teach users how to
do that either.
How are users supposed to cope with this craziness? Are they expected to
write their own scripts and understand this stuff?
As far as I can see, there are no applications out there at the moment that
come close to understanding how to configure a media pipeline, so users
have to understand how to use media-ctl to configure the pipeline manually.
Are we really expecting users to write scripts to do this, and understand
all these nuances?
IMHO, this is completely crazy, and hasn't been fully thought out.
> > $ 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:
>
> The origins of the S_FRAME_INTERVAL IOCTL for sub-devices are the S_PARM
> IOCTL for video nodes. It is used to control the frame rate for more simple
> devices that do not expose the Media controller interface. The similar
> S_FRAME_INTERVAL was added for sub-devices as well, and it has been so far
> used to control the frame interval for sensors (and G_FRAME_INTERVAL to
> obtain the frame interval for TV tuners, for instance).
>
> The pad argument was added there but media-ctl only supported setting the
> frame interval on pad 0, which, coincidentally, worked well for sensor
> devices.
>
> The link validation is primarily done in order to ensure the validity of the
> hardware configuration: streaming may not be started if the hardware
> configuration is not valid.
>
> Also, frame interval is not a static property during streaming: it may be
> changed without the knowledge of the other sub-device drivers downstream. It
> neither is a property of hardware receiving or processing images: if there
> are limitations in processing pixels, then they in practice are related to
> pixel rates or image sizes (i.e. not frame rates).
So what about the case where we have a subdev (CSI) that is capable of
frame rate reduction, that needs to know the input frame rate and the
desired output frame rate? It seems to me that this has not been
thought through...
> > - ``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.
>
> Skipping in the context is used to refer to sub-sampling. The term is often
> used in conjunction of sensors. The documentation could certainly be
> clarified here.
It definitely needs to be, it's currently mis-leading.
> > 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.
> >
> > In any case, the documentation vs media-ctl create something of a very
> > obscure situation, one that probably needs solving one way or another.
>
> Before going to solutions I need to ask: what do you want to achieve?
Full and consistent support for the hardware, and a sane and consistent
way to setup a media pipeline that is easy for everyone to understand.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Sakari Ailus <sakari.ailus@iki.fi> |
|---|---|
| Date | 2017-02-21 16:40 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdkHN-4r9-57@gated-at.bofh.it> |
| In reply to | #1585332 |
Hi Russell,
On Tue, Feb 21, 2017 at 01:21:32PM +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 21, 2017 at 02:37:57PM +0200, Sakari Ailus wrote:
> > Hi Russell,
> >
> > On Tue, Feb 21, 2017 at 12:13:32AM +0000, 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:
> >
> > The CSI-2 receivers typically do not implement frame interval IOCTLs as they
> > do not control the frame interval. Some sensors or TV tuners typically do,
> > so they implement these IOCTLs.
>
> No, TV tuners do not. The frame rate for a TV tuner is set by the
> broadcaster, not by the tuner. The tuner can't change that frame rate.
> The tuner may opt to "skip" fields or frames. That's no different from
> what the CSI block in my example below is capable of doing.
>
> Treating a tuner differently from the CSI block is inconsistent and
> completely wrong.
I agree tuners in that sense are somewhat similar, and they are not treated
differently because they are tuners (and not CSI-2 receivers). Neither can
control the frame rate of the incoming video stream.
Conceivably a tuner could implement G_FRAME_INTERVAL IOCTL, but based on a
quick glance none appears to. Neither do CSI-2 receivers. Only sensor
drivers do currently.
>
> > There are alternative ways to specify the frame rate.
>
> Empty statements (or hand-waving type statements) I'm afraid don't
> contribute to the discussion, because they mean nothing to me. Please
> give an example, or flesh out what you mean.
Images are transmitted as series of lines, with each line ending in a
horizontal blanking period, and each frame ending with a similar period of
vertical blanking. The blanking configuration in the units of pixels and
lines at their pixel clock is a native unit which sensors typically use, and
some drivers expose the blanking controls directly to the user.
<URL:http://hverkuil.home.xs4all.nl/spec/uapi/v4l/extended-controls.html#image-source-control-ids>
>
> > > $ 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:
> >
> > You should remove the frame interval setting from sub-devices that do not
> > support it.
>
> That means we end up with horribly complex scripts. This "solution" does
> not scale. Therefore, it is not a "solution".
I have to disagree with that: if a piece of hardware does not offer to
control or, if a concept is not even relevant for a piece of hardware, then
a driver for that piece of hardware should not expose an interface to
control such a feature. Doing so would provide no value and at the same time
would be simply confusing for the user space.
>
> It's fine if you want to write a script to setup the media pipeline using
> media-ctl, listing _each_ media-ctl command individually, with arguments
> specific to each step, but as I've already said, that does not scale.
Pipeline configuration as such is highly hardware specific. There are rules,
but there are details in hardware that have to be taken into account, such
as mandated cropping in certain situations. You have to simply accept that:
when it comes to camera Image Signal Processors, there are no standard
pipelines. Each ISP is different from the rest, more or less, and often more
so.
As the interface is generic, you can write generic programs that use that
interface, but you need to be able to adapt to the differences in the
functionality of the hardware.
Frankly, I think this is just needless noise stemming from a problem that's
not really difficult to solve --- if that technical problem really even
exists. But let's not debate that; I accept that dropping frames is what
you're willing to do.
>
> I don't want to end up writing separate scripts to configure the pipeline
> for different parameters or setups. I don't want to teach users how to
> do that either.
>
> How are users supposed to cope with this craziness? Are they expected to
> write their own scripts and understand this stuff?
>
> As far as I can see, there are no applications out there at the moment that
> come close to understanding how to configure a media pipeline, so users
> have to understand how to use media-ctl to configure the pipeline manually.
> Are we really expecting users to write scripts to do this, and understand
> all these nuances?
>
> IMHO, this is completely crazy, and hasn't been fully thought out.
>
> > > $ 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:
> >
> > The origins of the S_FRAME_INTERVAL IOCTL for sub-devices are the S_PARM
> > IOCTL for video nodes. It is used to control the frame rate for more simple
> > devices that do not expose the Media controller interface. The similar
> > S_FRAME_INTERVAL was added for sub-devices as well, and it has been so far
> > used to control the frame interval for sensors (and G_FRAME_INTERVAL to
> > obtain the frame interval for TV tuners, for instance).
> >
> > The pad argument was added there but media-ctl only supported setting the
> > frame interval on pad 0, which, coincidentally, worked well for sensor
> > devices.
> >
> > The link validation is primarily done in order to ensure the validity of the
> > hardware configuration: streaming may not be started if the hardware
> > configuration is not valid.
> >
> > Also, frame interval is not a static property during streaming: it may be
> > changed without the knowledge of the other sub-device drivers downstream. It
> > neither is a property of hardware receiving or processing images: if there
> > are limitations in processing pixels, then they in practice are related to
> > pixel rates or image sizes (i.e. not frame rates).
>
> So what about the case where we have a subdev (CSI) that is capable of
> frame rate reduction, that needs to know the input frame rate and the
> desired output frame rate? It seems to me that this has not been
> thought through...
That's because I believe you're the first one wanting to willfully throw
away perfectly good frames. :-)
If you want to do that, a simple option could be to just support
[GS]_FRAME_INTERVAL on all pads of a sub-device that can drop frames. But it
should not be include in pipeline validation, it simply does not belong
there for the reasons stated previously.
The user would be responsible for configuring the frame rates right. That
information would simply be used to configure frame dropping frequency.
I'd like to have a comment from Laurent and Hans on this.
>
> > > - ``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.
> >
> > Skipping in the context is used to refer to sub-sampling. The term is often
> > used in conjunction of sensors. The documentation could certainly be
> > clarified here.
>
> It definitely needs to be, it's currently mis-leading.
If you're not familiar with terminology typically used with many camera
sensor, perhaps so. The documentation should indeed not assume that; I'll
submit a patch to fix that.
>
> > > 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.
> > >
> > > In any case, the documentation vs media-ctl create something of a very
> > > obscure situation, one that probably needs solving one way or another.
> >
> > Before going to solutions I need to ask: what do you want to achieve?
>
> Full and consistent support for the hardware, and a sane and consistent
> way to setup a media pipeline that is easy for everyone to understand.
That's essentially what we do have: the same interface is supported on a
large variety of different hardware devices. However, not all IOCTLs are
supported by all device drivers.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
[toc] | [prev] | [next] | [standalone]
| From | Russell King - ARM Linux <linux@armlinux.org.uk> |
|---|---|
| Date | 2017-02-21 17:10 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdlaN-4Vr-3@gated-at.bofh.it> |
| In reply to | #1585434 |
On Tue, Feb 21, 2017 at 05:38:34PM +0200, Sakari Ailus wrote: > Hi Russell, > > On Tue, Feb 21, 2017 at 01:21:32PM +0000, Russell King - ARM Linux wrote: > > On Tue, Feb 21, 2017 at 02:37:57PM +0200, Sakari Ailus wrote: > > > Hi Russell, > > > > > > On Tue, Feb 21, 2017 at 12:13:32AM +0000, 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: > > > > > > The CSI-2 receivers typically do not implement frame interval IOCTLs as they > > > do not control the frame interval. Some sensors or TV tuners typically do, > > > so they implement these IOCTLs. > > > > No, TV tuners do not. The frame rate for a TV tuner is set by the > > broadcaster, not by the tuner. The tuner can't change that frame rate. > > The tuner may opt to "skip" fields or frames. That's no different from > > what the CSI block in my example below is capable of doing. > > > > Treating a tuner differently from the CSI block is inconsistent and > > completely wrong. > > I agree tuners in that sense are somewhat similar, and they are not treated > differently because they are tuners (and not CSI-2 receivers). Neither can > control the frame rate of the incoming video stream. > > Conceivably a tuner could implement G_FRAME_INTERVAL IOCTL, but based on a > quick glance none appears to. Neither do CSI-2 receivers. Only sensor > drivers do currently. Please look again. I am being very careful with "CSI" vs "CSI-2" in my emails, you are conflating the two. In all my emails so far, "CSI" refers to a block of hardware that is responsible for receiving an image stream from some kind of source. It contains hardware that supports frame skipping. "CSI-2" refers to a different block of hardware that is responsible for receiving a serially encoded stream from a MIPI-CSI-2 compliant source and providing it to the "CSI" block. I would have thought my diagram that I drew would have made it clear that they were different blocks of hardware, but I guess in this case, the old saying "a picture is worth 1000 words" is simply not true. > Images are transmitted as series of lines, with each line ending in a > horizontal blanking period, and each frame ending with a similar period of I'm sorry, are you seriously teaching me to suck rocks? I am insulted. I've been involved in TV and video for many years, I don't need you to tell me how video is transmitted. Sorry, you've just lost my interest in further discussion. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net.
[toc] | [prev] | [next] | [standalone]
| From | Sakari Ailus <sakari.ailus@iki.fi> |
|---|---|
| Date | 2017-02-21 17:20 +0100 |
| Subject | Re: [PATCH v4 29/36] media: imx: mipi-csi2: enable setting and getting of frame rates |
| Message-ID | <tdlkt-4YS-7@gated-at.bofh.it> |
| In reply to | #1585477 |
On Tue, Feb 21, 2017 at 04:03:32PM +0000, Russell King - ARM Linux wrote: > On Tue, Feb 21, 2017 at 05:38:34PM +0200, Sakari Ailus wrote: > > Hi Russell, > > > > On Tue, Feb 21, 2017 at 01:21:32PM +0000, Russell King - ARM Linux wrote: > > > On Tue, Feb 21, 2017 at 02:37:57PM +0200, Sakari Ailus wrote: > > > > Hi Russell, > > > > > > > > On Tue, Feb 21, 2017 at 12:13:32AM +0000, 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: > > > > > > > > The CSI-2 receivers typically do not implement frame interval IOCTLs as they > > > > do not control the frame interval. Some sensors or TV tuners typically do, > > > > so they implement these IOCTLs. > > > > > > No, TV tuners do not. The frame rate for a TV tuner is set by the > > > broadcaster, not by the tuner. The tuner can't change that frame rate. > > > The tuner may opt to "skip" fields or frames. That's no different from > > > what the CSI block in my example below is capable of doing. > > > > > > Treating a tuner differently from the CSI block is inconsistent and > > > completely wrong. > > > > I agree tuners in that sense are somewhat similar, and they are not treated > > differently because they are tuners (and not CSI-2 receivers). Neither can > > control the frame rate of the incoming video stream. > > > > Conceivably a tuner could implement G_FRAME_INTERVAL IOCTL, but based on a > > quick glance none appears to. Neither do CSI-2 receivers. Only sensor > > drivers do currently. > > Please look again. I am being very careful with "CSI" vs "CSI-2" in my > emails, you are conflating the two. > > In all my emails so far, "CSI" refers to a block of hardware that is > responsible for receiving an image stream from some kind of source. It > contains hardware that supports frame skipping. Ah, I missed the difference. Thanks for pointing it out. Still, that does not change how the skipping would work nor how I proposed it would be configured from the user space. > > "CSI-2" refers to a different block of hardware that is responsible for > receiving a serially encoded stream from a MIPI-CSI-2 compliant source > and providing it to the "CSI" block. > > I would have thought my diagram that I drew would have made it clear that > they were different blocks of hardware, but I guess in this case, the old > saying "a picture is worth 1000 words" is simply not true. > > > Images are transmitted as series of lines, with each line ending in a > > horizontal blanking period, and each frame ending with a similar period of > > I'm sorry, are you seriously teaching me to suck rocks? I am insulted. > > I've been involved in TV and video for many years, I don't need you to > tell me how video is transmitted. > > Sorry, you've just lost my interest in further discussion. There's no need to feel insulted; that certainly was not the intention. I've proposed you a solution, please comment on that. -- Regards, Sakari Ailus e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-16 03:30 +0100 |
| Subject | [PATCH v4 13/36] [media] v4l2: add a frame timeout event |
| Message-ID | <tbjZx-8cO-59@gated-at.bofh.it> |
| In reply to | #1582252 |
Add a new FRAME_TIMEOUT event to signal that a video capture or
output device has timed out waiting for reception or transmit
completion of a video frame.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
Documentation/media/uapi/v4l/vidioc-dqevent.rst | 5 +++++
Documentation/media/videodev2.h.rst.exceptions | 1 +
include/uapi/linux/videodev2.h | 1 +
3 files changed, 7 insertions(+)
diff --git a/Documentation/media/uapi/v4l/vidioc-dqevent.rst b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
index 8d663a7..dd77d9b 100644
--- a/Documentation/media/uapi/v4l/vidioc-dqevent.rst
+++ b/Documentation/media/uapi/v4l/vidioc-dqevent.rst
@@ -197,6 +197,11 @@ call.
the regions changes. This event has a struct
:c:type:`v4l2_event_motion_det`
associated with it.
+ * - ``V4L2_EVENT_FRAME_TIMEOUT``
+ - 7
+ - This event is triggered when the video capture or output device
+ has timed out waiting for the reception or transmit completion of
+ a frame of video.
* - ``V4L2_EVENT_PRIVATE_START``
- 0x08000000
- Base event number for driver-private events.
diff --git a/Documentation/media/videodev2.h.rst.exceptions b/Documentation/media/videodev2.h.rst.exceptions
index e11a0d0..5b0f767 100644
--- a/Documentation/media/videodev2.h.rst.exceptions
+++ b/Documentation/media/videodev2.h.rst.exceptions
@@ -459,6 +459,7 @@ replace define V4L2_EVENT_CTRL event-type
replace define V4L2_EVENT_FRAME_SYNC event-type
replace define V4L2_EVENT_SOURCE_CHANGE event-type
replace define V4L2_EVENT_MOTION_DET event-type
+replace define V4L2_EVENT_FRAME_TIMEOUT event-type
replace define V4L2_EVENT_PRIVATE_START event-type
replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 46e8a2e3..e174c45 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -2132,6 +2132,7 @@ struct v4l2_streamparm {
#define V4L2_EVENT_FRAME_SYNC 4
#define V4L2_EVENT_SOURCE_CHANGE 5
#define V4L2_EVENT_MOTION_DET 6
+#define V4L2_EVENT_FRAME_TIMEOUT 7
#define V4L2_EVENT_PRIVATE_START 0x08000000
/* Payload for V4L2_EVENT_VSYNC */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-16 03:30 +0100 |
| Subject | [PATCH v4 31/36] media: imx: csi: add __csi_get_fmt |
| Message-ID | <tbjZx-8cO-51@gated-at.bofh.it> |
| In reply to | #1582252 |
Add __csi_get_fmt() and use it to return the correct mbus format
(active or try) in get_fmt. Use it in other places as well.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
Suggested-by: Russell King <linux@armlinux.org.uk>
---
drivers/staging/media/imx/imx-media-csi.c | 52 ++++++++++++++++++++++++-------
1 file changed, 40 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c
index 63555dc..b0aac82 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -788,7 +788,20 @@ static int csi_eof_isr(struct v4l2_subdev *sd, u32 status, bool *handled)
return 0;
}
-static int csi_try_crop(struct csi_priv *priv, struct v4l2_rect *crop,
+static struct v4l2_mbus_framefmt *
+__csi_get_fmt(struct csi_priv *priv, struct v4l2_subdev_pad_config *cfg,
+ unsigned int pad, enum v4l2_subdev_format_whence which)
+{
+ if (which == V4L2_SUBDEV_FORMAT_TRY)
+ return v4l2_subdev_get_try_format(&priv->sd, cfg, pad);
+ else
+ return &priv->format_mbus[pad];
+}
+
+static int csi_try_crop(struct csi_priv *priv,
+ struct v4l2_rect *crop,
+ struct v4l2_subdev_pad_config *cfg,
+ enum v4l2_subdev_format_whence which,
struct imx_media_subdev *sensor)
{
struct v4l2_of_endpoint *sensor_ep;
@@ -796,7 +809,7 @@ static int csi_try_crop(struct csi_priv *priv, struct v4l2_rect *crop,
v4l2_std_id std;
int ret;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
+ infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, which);
sensor_ep = &sensor->sensor_ep;
crop->width = min_t(__u32, infmt->width, crop->width);
@@ -852,11 +865,16 @@ static int csi_get_fmt(struct v4l2_subdev *sd,
struct v4l2_subdev_format *sdformat)
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
+ struct v4l2_mbus_framefmt *fmt;
if (sdformat->pad >= CSI_NUM_PADS)
return -EINVAL;
- sdformat->format = priv->format_mbus[sdformat->pad];
+ fmt = __csi_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
+ if (!fmt)
+ return -EINVAL;
+
+ sdformat->format = *fmt;
return 0;
}
@@ -880,8 +898,6 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
if (priv->stream_on)
return -EBUSY;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
-
sensor = imx_media_find_sensor(priv->md, &priv->sd.entity);
if (IS_ERR(sensor)) {
v4l2_err(&priv->sd, "no sensor attached\n");
@@ -895,6 +911,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
switch (sdformat->pad) {
case CSI_SRC_PAD_DIRECT:
case CSI_SRC_PAD_IDMAC:
+ infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sdformat->which);
+
if (sdformat->format.width < priv->crop.width * 3 / 4)
sdformat->format.width = priv->crop.width / 2;
else
@@ -957,7 +975,8 @@ static int csi_set_fmt(struct v4l2_subdev *sd,
crop.top = 0;
crop.width = sdformat->format.width;
crop.height = sdformat->format.height;
- ret = csi_try_crop(priv, &crop, sensor);
+ ret = csi_try_crop(priv, &crop, cfg,
+ sdformat->which, sensor);
if (ret)
return ret;
@@ -1004,7 +1023,9 @@ static int csi_get_selection(struct v4l2_subdev *sd,
if (sel->pad >= CSI_NUM_PADS || sel->pad == CSI_SINK_PAD)
return -EINVAL;
- infmt = &priv->format_mbus[CSI_SINK_PAD];
+ infmt = __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sel->which);
+ if (!infmt)
+ return -EINVAL;
switch (sel->target) {
case V4L2_SEL_TGT_CROP_BOUNDS:
@@ -1014,7 +1035,14 @@ static int csi_get_selection(struct v4l2_subdev *sd,
sel->r.height = infmt->height;
break;
case V4L2_SEL_TGT_CROP:
- sel->r = priv->crop;
+ if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
+ struct v4l2_rect *try_crop =
+ v4l2_subdev_get_try_crop(&priv->sd,
+ cfg, sel->pad);
+ sel->r = *try_crop;
+ } else {
+ sel->r = priv->crop;
+ }
break;
default:
return -EINVAL;
@@ -1028,7 +1056,6 @@ static int csi_set_selection(struct v4l2_subdev *sd,
struct v4l2_subdev_selection *sel)
{
struct csi_priv *priv = v4l2_get_subdevdata(sd);
- struct v4l2_mbus_framefmt *outfmt;
struct imx_media_subdev *sensor;
int ret;
@@ -1058,15 +1085,16 @@ static int csi_set_selection(struct v4l2_subdev *sd,
return 0;
}
- outfmt = &priv->format_mbus[sel->pad];
-
- ret = csi_try_crop(priv, &sel->r, sensor);
+ ret = csi_try_crop(priv, &sel->r, cfg, sel->which, sensor);
if (ret)
return ret;
if (sel->which == V4L2_SUBDEV_FORMAT_TRY) {
cfg->try_crop = sel->r;
} else {
+ struct v4l2_mbus_framefmt *outfmt =
+ &priv->format_mbus[sel->pad];
+
priv->crop = sel->r;
/* Update the source format */
outfmt->width = sel->r.width;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-16 03:30 +0100 |
| Subject | [PATCH v4 12/36] add mux and video interface bridge entity functions |
| Message-ID | <tbjZx-8cO-55@gated-at.bofh.it> |
| In reply to | #1582252 |
From: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
- renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
Documentation/media/uapi/mediactl/media-types.rst | 22 ++++++++++++++++++++++
include/uapi/linux/media.h | 6 ++++++
2 files changed, 28 insertions(+)
diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst
index 3e03dc2..023be29 100644
--- a/Documentation/media/uapi/mediactl/media-types.rst
+++ b/Documentation/media/uapi/mediactl/media-types.rst
@@ -298,6 +298,28 @@ Types and flags used to represent the media graph elements
received on its sink pad and outputs the statistics data on
its source pad.
+ - .. row 29
+
+ .. _MEDIA-ENT-F-MUX:
+
+ - ``MEDIA_ENT_F_MUX``
+
+ - Video multiplexer. An entity capable of multiplexing must have at
+ least two sink pads and one source pad, and must pass the video
+ frame(s) received from the active sink pad to the source pad. Video
+ frame(s) from the inactive sink pads are discarded.
+
+ - .. row 30
+
+ .. _MEDIA-ENT-F-VID-IF-BRIDGE:
+
+ - ``MEDIA_ENT_F_VID_IF_BRIDGE``
+
+ - Video interface bridge. A video interface bridge entity must have at
+ least one sink pad and one source pad. It receives video frame(s) on
+ its sink pad in one bus format (HDMI, eDP, MIPI CSI-2, ...) and
+ converts them and outputs them on its source pad in another bus format
+ (eDP, MIPI CSI-2, parallel, ...).
.. tabularcolumns:: |p{5.5cm}|p{12.0cm}|
diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
index 4890787..fac96c6 100644
--- a/include/uapi/linux/media.h
+++ b/include/uapi/linux/media.h
@@ -105,6 +105,12 @@ struct media_device_info {
#define MEDIA_ENT_F_PROC_VIDEO_STATISTICS (MEDIA_ENT_F_BASE + 0x4006)
/*
+ * Switch and bridge entitites
+ */
+#define MEDIA_ENT_F_VID_MUX (MEDIA_ENT_F_BASE + 0x5001)
+#define MEDIA_ENT_F_VID_IF_BRIDGE (MEDIA_ENT_F_BASE + 0x5002)
+
+/*
* Connectors
*/
/* It is a responsibility of the entity drivers to add connectors and links */
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-02-19 22:40 +0100 |
| Subject | Re: [PATCH v4 12/36] add mux and video interface bridge entity functions |
| Message-ID | <tcHn3-47I-5@gated-at.bofh.it> |
| In reply to | #1582267 |
[Multipart message — attachments visible in raw view] — view raw
On Wed 2017-02-15 18:19:14, Steve Longerbeam wrote: > From: Philipp Zabel <p.zabel@pengutronix.de> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> > > - renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX > > Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com> This is slightly "interesting" format of changelog. Normally signoffs go below. > diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst > index 3e03dc2..023be29 100644 > --- a/Documentation/media/uapi/mediactl/media-types.rst > +++ b/Documentation/media/uapi/mediactl/media-types.rst > @@ -298,6 +298,28 @@ Types and flags used to represent the media graph elements > received on its sink pad and outputs the statistics data on > its source pad. > > + - .. row 29 > + > + .. _MEDIA-ENT-F-MUX: > + > + - ``MEDIA_ENT_F_MUX`` And you probably want to rename it here, too. With that fixed: Reviewed-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-22 18:30 +0100 |
| Subject | Re: [PATCH v4 12/36] add mux and video interface bridge entity functions |
| Message-ID | <tdITL-57Y-5@gated-at.bofh.it> |
| In reply to | #1584259 |
On 02/19/2017 01:28 PM, Pavel Machek wrote: > On Wed 2017-02-15 18:19:14, Steve Longerbeam wrote: >> From: Philipp Zabel <p.zabel@pengutronix.de> >> >> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de> >> >> - renamed MEDIA_ENT_F_MUX to MEDIA_ENT_F_VID_MUX >> >> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com> > > This is slightly "interesting" format of changelog. Normally signoffs > go below. > >> diff --git a/Documentation/media/uapi/mediactl/media-types.rst b/Documentation/media/uapi/mediactl/media-types.rst >> index 3e03dc2..023be29 100644 >> --- a/Documentation/media/uapi/mediactl/media-types.rst >> +++ b/Documentation/media/uapi/mediactl/media-types.rst >> @@ -298,6 +298,28 @@ Types and flags used to represent the media graph elements >> received on its sink pad and outputs the statistics data on >> its source pad. >> >> + - .. row 29 >> + >> + .. _MEDIA-ENT-F-MUX: >> + >> + - ``MEDIA_ENT_F_MUX`` > > And you probably want to rename it here, too. Done, thanks. Steve > > With that fixed: > > Reviewed-by: Pavel Machek <pavel@ucw.cz> >
[toc] | [prev] | [next] | [standalone]
| From | Steve Longerbeam <slongerbeam@gmail.com> |
|---|---|
| Date | 2017-02-16 03:30 +0100 |
| Subject | [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline |
| Message-ID | <tbjZx-8cO-67@gated-at.bofh.it> |
| In reply to | #1582252 |
v4l2_pipeline_inherit_controls() will add the v4l2 controls from
all subdev entities in a pipeline to a given video device.
Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
---
drivers/media/v4l2-core/v4l2-mc.c | 48 +++++++++++++++++++++++++++++++++++++++
include/media/v4l2-mc.h | 25 ++++++++++++++++++++
2 files changed, 73 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
index 303980b..09d4d97 100644
--- a/drivers/media/v4l2-core/v4l2-mc.c
+++ b/drivers/media/v4l2-core/v4l2-mc.c
@@ -22,6 +22,7 @@
#include <linux/usb.h>
#include <media/media-device.h>
#include <media/media-entity.h>
+#include <media/v4l2-ctrls.h>
#include <media/v4l2-fh.h>
#include <media/v4l2-mc.h>
#include <media/v4l2-subdev.h>
@@ -238,6 +239,53 @@ int v4l_vb2q_enable_media_source(struct vb2_queue *q)
}
EXPORT_SYMBOL_GPL(v4l_vb2q_enable_media_source);
+int __v4l2_pipeline_inherit_controls(struct video_device *vfd,
+ struct media_entity *start_entity)
+{
+ struct media_device *mdev = start_entity->graph_obj.mdev;
+ struct media_entity *entity;
+ struct media_graph graph;
+ struct v4l2_subdev *sd;
+ int ret;
+
+ ret = media_graph_walk_init(&graph, mdev);
+ if (ret)
+ return ret;
+
+ media_graph_walk_start(&graph, start_entity);
+
+ while ((entity = media_graph_walk_next(&graph))) {
+ if (!is_media_entity_v4l2_subdev(entity))
+ continue;
+
+ sd = media_entity_to_v4l2_subdev(entity);
+
+ ret = v4l2_ctrl_add_handler(vfd->ctrl_handler,
+ sd->ctrl_handler,
+ NULL);
+ if (ret)
+ break;
+ }
+
+ media_graph_walk_cleanup(&graph);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__v4l2_pipeline_inherit_controls);
+
+int v4l2_pipeline_inherit_controls(struct video_device *vfd,
+ struct media_entity *start_entity)
+{
+ struct media_device *mdev = start_entity->graph_obj.mdev;
+ int ret;
+
+ mutex_lock(&mdev->graph_mutex);
+ ret = __v4l2_pipeline_inherit_controls(vfd, start_entity);
+ mutex_unlock(&mdev->graph_mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_pipeline_inherit_controls);
+
/* -----------------------------------------------------------------------------
* Pipeline power management
*
diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
index 2634d9d..9848e77 100644
--- a/include/media/v4l2-mc.h
+++ b/include/media/v4l2-mc.h
@@ -171,6 +171,17 @@ void v4l_disable_media_source(struct video_device *vdev);
*/
int v4l_vb2q_enable_media_source(struct vb2_queue *q);
+/**
+ * v4l2_pipeline_inherit_controls - Add the v4l2 controls from all
+ * subdev entities in a pipeline to
+ * the given video device.
+ * @vfd: the video device
+ * @start_entity: Starting entity
+ */
+int __v4l2_pipeline_inherit_controls(struct video_device *vfd,
+ struct media_entity *start_entity);
+int v4l2_pipeline_inherit_controls(struct video_device *vfd,
+ struct media_entity *start_entity);
/**
* v4l2_pipeline_pm_use - Update the use count of an entity
@@ -231,6 +242,20 @@ static inline int v4l_vb2q_enable_media_source(struct vb2_queue *q)
return 0;
}
+static inline int __v4l2_pipeline_inherit_controls(
+ struct video_device *vfd,
+ struct media_entity *start_entity)
+{
+ return 0;
+}
+
+static inline int v4l2_pipeline_inherit_controls(
+ struct video_device *vfd,
+ struct media_entity *start_entity)
+{
+ return 0;
+}
+
static inline int v4l2_pipeline_pm_use(struct media_entity *entity, int use)
{
return 0;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Pavel Machek <pavel@ucw.cz> |
|---|---|
| Date | 2017-02-19 23:00 +0100 |
| Subject | Re: [PATCH v4 14/36] [media] v4l2-mc: add a function to inherit controls from a pipeline |
| Message-ID | <tcHGq-4eE-19@gated-at.bofh.it> |
| In reply to | #1582268 |
[Multipart message — attachments visible in raw view] — view raw
On Wed 2017-02-15 18:19:16, Steve Longerbeam wrote: > v4l2_pipeline_inherit_controls() will add the v4l2 controls from > all subdev entities in a pipeline to a given video device. > > Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com> Reviewed-by: Pavel Machek <pavel@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[toc] | [prev] | [next] | [standalone]
Page 3 of 5 — ← Prev page 1 2 [3] 4 5 Next page →
Back to top | Article view | linux.kernel
csiph-web