Re: Shawn Guo: your attetion is needed here Re: [PATCH v8 00/34] i.MX Media Driver
On Tue 2017-06-20 08:05:05, Fabio Estevam wrote: > On Tue, Jun 20, 2017 at 5:29 AM, Pavel Machekwrote: > > > Hmm. I changed the subject to grab Shawn's attetion. > > > > But his acks should not be needed for forward progress. Yes, it would > > be good, but he does not react -- so just reorder the series so that > > dts changes come last, then apply the parts you can apply: driver can > > go in. > > > > And actually... if maintainer does not respond at all, there are ways > > to deal with that, too... > > Shawn has already applied the dts part of the series and they show up > in linux-next. Aha, sorry about the noise. I see videomux parts being merged by Mauro. Good :-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: Shawn Guo: your attetion is needed here Re: [PATCH v8 00/34] i.MX Media Driver
On Tue, Jun 20, 2017 at 5:29 AM, Pavel Machekwrote: > Hmm. I changed the subject to grab Shawn's attetion. > > But his acks should not be needed for forward progress. Yes, it would > be good, but he does not react -- so just reorder the series so that > dts changes come last, then apply the parts you can apply: driver can > go in. > > And actually... if maintainer does not respond at all, there are ways > to deal with that, too... Shawn has already applied the dts part of the series and they show up in linux-next.
Shawn Guo: your attetion is needed here Re: [PATCH v8 00/34] i.MX Media Driver
Hi! > >> But as Pavel pointed out, in fact we are missing many > >> Acks still, for all of the dts source changes (patches > >> 4-14), as well as really everything else (imx-media staging > >> driver patches). > > > > No Acks needed for the staging part. It's staging, so not held > > to the same standards as non-staging parts. That doesn't mean > > Acks aren't welcome, of course. > > Acks are wanted for particular i.MX DTS changes including device > tree binding descriptions. > > Shawn, please bless the series. Hmm. I changed the subject to grab Shawn's attetion. But his acks should not be needed for forward progress. Yes, it would be good, but he does not react -- so just reorder the series so that dts changes come last, then apply the parts you can apply: driver can go in. And actually... if maintainer does not respond at all, there are ways to deal with that, too... Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/11/2017 01:05 PM, Vladimir Zapolskiy wrote: On 06/10/2017 02:26 AM, Hans Verkuil wrote: On 10/06/17 01:16, Steve Longerbeam wrote: On 06/07/2017 12:02 PM, Hans Verkuil wrote: We're still waiting for an Ack for patch 02/34, right? Hi Hans, Rub has provided an Ack for patch 2. Other than that everything is ready AFAICT. But as Pavel pointed out, in fact we are missing many Acks still, for all of the dts source changes (patches 4-14), as well as really everything else (imx-media staging driver patches). No Acks needed for the staging part. It's staging, so not held to the same standards as non-staging parts. That doesn't mean Acks aren't welcome, of course. Acks are wanted for particular i.MX DTS changes including device tree binding descriptions. Shawn, please bless the series. I second that request! There are a couple minor update to the ARM dts patches for imx6, I will post a new series for them. But there should not be anything of much controversy in these ARM dts patches. Steve
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/10/2017 02:26 AM, Hans Verkuil wrote: > On 10/06/17 01:16, Steve Longerbeam wrote: >> >> >> On 06/07/2017 12:02 PM, Hans Verkuil wrote: >>> We're still waiting for an Ack for patch 02/34, right? >>> >> >> Hi Hans, Rub has provided an Ack for patch 2. >> >>> Other than that everything is ready AFAICT. >>> >> >> But as Pavel pointed out, in fact we are missing many >> Acks still, for all of the dts source changes (patches >> 4-14), as well as really everything else (imx-media staging >> driver patches). > > No Acks needed for the staging part. It's staging, so not held > to the same standards as non-staging parts. That doesn't mean > Acks aren't welcome, of course. Acks are wanted for particular i.MX DTS changes including device tree binding descriptions. Shawn, please bless the series. > > You don't need Greg's Ack for staging/media either, patches there > go in via us (generally at least) and we handle those, not Greg. > -- With best wishes, Vladimir
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/11/2017 10:28 AM, Hans Verkuil wrote: Hi Steve, Philipp, While preparing the pull request I noticed that the MAINTAINERS file wasn't updated. Steve, can you post a patch adding entries for the imx and ov5640 driver? Philipp, can you do the same for the video mux? I assume you're the maintainer for this? Thanks! I also made it possible to compile-test this driver with this patch: Signed-off-by: Hans Verkuil--- diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index 7eff50bcea39..22f968cf32b1 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -1,6 +1,7 @@ config VIDEO_IMX_MEDIA tristate "i.MX5/6 V4L2 media core driver" - depends on MEDIA_CONTROLLER && VIDEO_V4L2 && ARCH_MXC && IMX_IPUV3_CORE + depends on MEDIA_CONTROLLER && VIDEO_V4L2 + depends on (ARCH_MXC && IMX_IPUV3_CORE) || COMPILE_TEST select V4L2_FWNODE ---help--- Say yes here to enable support for video4linux media controller This is too optimistic, it fails on IPU code. Ignore this chunk. Hans diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index c306146a4247..a2d26693912e 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include Steve, if you're OK with that I was planning to just modify your original patch rather than adding another patch on top. Regards, Hans
Re: [PATCH v8 00/34] i.MX Media Driver
Hi Steve, Philipp, While preparing the pull request I noticed that the MAINTAINERS file wasn't updated. Steve, can you post a patch adding entries for the imx and ov5640 driver? Philipp, can you do the same for the video mux? I assume you're the maintainer for this? Thanks! I also made it possible to compile-test this driver with this patch: Signed-off-by: Hans Verkuil--- diff --git a/drivers/staging/media/imx/Kconfig b/drivers/staging/media/imx/Kconfig index 7eff50bcea39..22f968cf32b1 100644 --- a/drivers/staging/media/imx/Kconfig +++ b/drivers/staging/media/imx/Kconfig @@ -1,6 +1,7 @@ config VIDEO_IMX_MEDIA tristate "i.MX5/6 V4L2 media core driver" - depends on MEDIA_CONTROLLER && VIDEO_V4L2 && ARCH_MXC && IMX_IPUV3_CORE + depends on MEDIA_CONTROLLER && VIDEO_V4L2 + depends on (ARCH_MXC && IMX_IPUV3_CORE) || COMPILE_TEST select V4L2_FWNODE ---help--- Say yes here to enable support for video4linux media controller diff --git a/drivers/staging/media/imx/imx-media-csi.c b/drivers/staging/media/imx/imx-media-csi.c index c306146a4247..a2d26693912e 100644 --- a/drivers/staging/media/imx/imx-media-csi.c +++ b/drivers/staging/media/imx/imx-media-csi.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include Steve, if you're OK with that I was planning to just modify your original patch rather than adding another patch on top. Regards, Hans
Re: [PATCH v8 00/34] i.MX Media Driver
Hi! > >> Other than that everything is ready AFAICT. > >> > > > > But as Pavel pointed out, in fact we are missing many > > Acks still, for all of the dts source changes (patches > > 4-14), as well as really everything else (imx-media staging > > driver patches). > > No Acks needed for the staging part. It's staging, so not held > to the same standards as non-staging parts. That doesn't mean > Acks aren't welcome, of course. > > You don't need Greg's Ack for staging/media either, patches there > go in via us (generally at least) and we handle those, not Greg. Ok, good. Can you just apply the patches? This is staging, they can be reviewed/fixed there -- as expected for staging. They are already way beyond staging quality, we don't want to get to "series v17" here, and repeatedly sending them over the email does not really do them any good. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v8 00/34] i.MX Media Driver
On 10/06/17 01:16, Steve Longerbeam wrote: > > > On 06/07/2017 12:02 PM, Hans Verkuil wrote: >> We're still waiting for an Ack for patch 02/34, right? >> > > Hi Hans, Rub has provided an Ack for patch 2. > >> Other than that everything is ready AFAICT. >> > > But as Pavel pointed out, in fact we are missing many > Acks still, for all of the dts source changes (patches > 4-14), as well as really everything else (imx-media staging > driver patches). No Acks needed for the staging part. It's staging, so not held to the same standards as non-staging parts. That doesn't mean Acks aren't welcome, of course. You don't need Greg's Ack for staging/media either, patches there go in via us (generally at least) and we handle those, not Greg. Regards, Hans
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/09/2017 04:16 PM, Steve Longerbeam wrote: On 06/07/2017 12:02 PM, Hans Verkuil wrote: We're still waiting for an Ack for patch 02/34, right? Hi Hans, Rub damn, I really need to proof-read before hitting send. "Rob" (sorry Rob!). Steve
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/07/2017 12:02 PM, Hans Verkuil wrote: We're still waiting for an Ack for patch 02/34, right? Hi Hans, Rub has provided an Ack for patch 2. Other than that everything is ready AFAICT. But as Pavel pointed out, in fact we are missing many Acks still, for all of the dts source changes (patches 4-14), as well as really everything else (imx-media staging driver patches). Steve
Re: [PATCH v8 00/34] i.MX Media Driver
On 06/07/2017 12:02 PM, Hans Verkuil wrote: We're still waiting for an Ack for patch 02/34, right? Hi Hans, Yes still waiting for an ack for the imx-media bindings. Other than that everything is ready AFAICT. Agreed. Steve Regards, Hans On 07/06/17 20:33, Steve Longerbeam wrote: In version 8: - Switched to v4l2_fwnode APIs. - Always pass a valid CSI id to ipu_set_ic_src_mux() in imx-ic-prp, even if the IC is receiving from the VDIC. The reason is due to a bug in the i.MX6 reference manual: from experiment it is determined that the CSI id select bit in IPU_CONF register selects which CSI is routed to either the VDIC or the IC, and is independent of whether the IC is set to receive from a CSI or the VDIC. Sugested by Marek Vasut. - ov5640: propagate error codes from all i2c register accesses. Sugested by Sakari Ailus . - ov5640: drop the entity stream count check in ov5640_s_stream(). Sugested by Sakari Ailus . - ov5640: Fix manual exposure control. The manual exposure setting (in line periods) cannot exceed the max exposure value in registers {0x380E, 0x380F} + {0x350C,0x350D}, however the max eposure value was not being calcualted correctly. - ov5640: the video mode register tables require auto gain/exposure be disabled before programming the register set. However auto gain/exp was being disabled by direct register write. This caused the auto gain/exp control values to be inconsistent with the actual hardware setting. Fixed by going through v4l2-ctrl when disabling auto gain/exp. - ov5640: converted virtual channel macro to a module parameter, default to channel 0. - ov5640: override the v4l2-ctl lock to use the ov5640 subdev driver's lock. Sugested by Sakari Ailus . - ov5640: switch to unit-less V4L2_CID_EXPOSURE. Using V4L2_CID_EXPOSURE_ABSOLUTE will require converting from 100-usec units to line periods and vice-versa. Sugested by Sakari Ailus and Pavel Machek . - ov5640: drop dangling regulator_bulk_disable() from probe/remove. Sugested by Sakari Ailus . - FIM: move input capture channel selection out of the device-tree and make this a V4L2 control. In order to support attaching a FIM to prpencvf, the FIM cannot have any device-tree configuration, because prpencvf has no device node. The FIM now is completely configurable via its V4L2 controls. - FIM: drop imx_media_fim_set_power(), and move the input capture channel request to imx_media_fim_set_stream(). This allows to drop csi_s_power() as well, since the latter only called imx_media_fim_set_power(). - FIM: add a spinlock to protect the frame_interval_monitor() from the setting of new control values. The frame_interval_monitor() is called from interrupt context so a spinlock must be used. - Updated to version 8 video-mux patchset from Philipp Zabel . Marek Vasut (1): media: imx: Drop warning upon multiple S_STREAM disable calls Philipp Zabel (8): dt-bindings: Add bindings for video-multiplexer device ARM: dts: imx6qdl: add multiplexer controls ARM: dts: imx6qdl: Add video multiplexers, mipi_csi, and their connections add mux and video interface bridge entity functions platform: add video-multiplexer subdevice driver media: imx: csi: increase burst size for YUV formats media: imx: csi: add frame skipping support media: imx: csi: add sink selection rectangles Russell King (3): media: imx: csi: add support for bayer formats media: imx: csi: add frame size/interval enumeration media: imx: capture: add frame sizes/interval enumeration Steve Longerbeam (22): [media] dt-bindings: Add bindings for i.MX media driver [media] dt/bindings: Add bindings for OV5640 ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node ARM: dts: imx6qdl: add capture-subsystem device ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors ARM: dts: imx6-sabreauto: create i2cmux for i2c3 ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture ARM: dts: imx6-sabreauto: add the ADV7180 video decoder [media] add Omnivision OV5640 sensor driver media: Add userspace header file for i.MX media: Add i.MX media core driver media: imx: Add a TODO file media: imx: Add Capture Device Interface media: imx: Add CSI subdev driver media: imx: Add VDIC subdev driver media: imx: Add IC subdev drivers media: imx: Add MIPI CSI-2 Receiver subdev driver media: imx: set and propagate default field, colorimetry ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers .../devicetree/bindings/media/i2c/ov5640.txt
Re: [PATCH v8 00/34] i.MX Media Driver
We're still waiting for an Ack for patch 02/34, right? Other than that everything is ready AFAICT. Regards, Hans On 07/06/17 20:33, Steve Longerbeam wrote: > In version 8: > > - Switched to v4l2_fwnode APIs. > > - Always pass a valid CSI id to ipu_set_ic_src_mux() in imx-ic-prp, even > if the IC is receiving from the VDIC. The reason is due to a bug in the > i.MX6 reference manual: from experiment it is determined that the CSI id > select bit in IPU_CONF register selects which CSI is routed to either > the VDIC or the IC, and is independent of whether the IC is set to > receive from a CSI or the VDIC. Sugested by Marek Vasut. > > - ov5640: propagate error codes from all i2c register accesses. > Sugested by Sakari Ailus . > > - ov5640: drop the entity stream count check in ov5640_s_stream(). > Sugested by Sakari Ailus . > > - ov5640: Fix manual exposure control. The manual exposure setting > (in line periods) cannot exceed the max exposure value in registers > {0x380E, 0x380F} + {0x350C,0x350D}, however the max eposure value was > not being calcualted correctly. > > - ov5640: the video mode register tables require auto gain/exposure be > disabled before programming the register set. However auto gain/exp was > being disabled by direct register write. This caused the auto gain/exp > control values to be inconsistent with the actual hardware setting. Fixed > by going through v4l2-ctrl when disabling auto gain/exp. > > - ov5640: converted virtual channel macro to a module parameter, default > to channel 0. > > - ov5640: override the v4l2-ctl lock to use the ov5640 subdev driver's > lock. Sugested by Sakari Ailus . > > - ov5640: switch to unit-less V4L2_CID_EXPOSURE. Using > V4L2_CID_EXPOSURE_ABSOLUTE will require converting from 100-usec units > to line periods and vice-versa. Sugested by Sakari Ailus and > Pavel Machek . > > - ov5640: drop dangling regulator_bulk_disable() from probe/remove. > Sugested by Sakari Ailus . > > - FIM: move input capture channel selection out of the device-tree and > make this a V4L2 control. In order to support attaching a FIM to prpencvf, > the FIM cannot have any device-tree configuration, because prpencvf has > no device node. The FIM now is completely configurable via its V4L2 > controls. > > - FIM: drop imx_media_fim_set_power(), and move the input capture channel > request to imx_media_fim_set_stream(). This allows to drop csi_s_power() > as well, since the latter only called imx_media_fim_set_power(). > > - FIM: add a spinlock to protect the frame_interval_monitor() from the > setting of new control values. The frame_interval_monitor() is called > from interrupt context so a spinlock must be used. > > - Updated to version 8 video-mux patchset from Philipp Zabel > . > > > Marek Vasut (1): > media: imx: Drop warning upon multiple S_STREAM disable calls > > Philipp Zabel (8): > dt-bindings: Add bindings for video-multiplexer device > ARM: dts: imx6qdl: add multiplexer controls > ARM: dts: imx6qdl: Add video multiplexers, mipi_csi, and their > connections > add mux and video interface bridge entity functions > platform: add video-multiplexer subdevice driver > media: imx: csi: increase burst size for YUV formats > media: imx: csi: add frame skipping support > media: imx: csi: add sink selection rectangles > > Russell King (3): > media: imx: csi: add support for bayer formats > media: imx: csi: add frame size/interval enumeration > media: imx: capture: add frame sizes/interval enumeration > > Steve Longerbeam (22): > [media] dt-bindings: Add bindings for i.MX media driver > [media] dt/bindings: Add bindings for OV5640 > ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node > ARM: dts: imx6qdl: add capture-subsystem device > ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround > ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors > ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors > ARM: dts: imx6-sabreauto: create i2cmux for i2c3 > ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b > ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture > ARM: dts: imx6-sabreauto: add the ADV7180 video decoder > [media] add Omnivision OV5640 sensor driver > media: Add userspace header file for i.MX > media: Add i.MX media core driver > media: imx: Add a TODO file > media: imx: Add Capture Device Interface > media: imx: Add CSI subdev driver > media: imx: Add VDIC subdev driver > media: imx: Add IC subdev drivers > media: imx: Add MIPI CSI-2 Receiver subdev driver > media: imx: set and propagate default field, colorimetry > ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers > > .../devicetree/bindings/media/i2c/ov5640.txt
[PATCH v8 00/34] i.MX Media Driver
In version 8: - Switched to v4l2_fwnode APIs. - Always pass a valid CSI id to ipu_set_ic_src_mux() in imx-ic-prp, even if the IC is receiving from the VDIC. The reason is due to a bug in the i.MX6 reference manual: from experiment it is determined that the CSI id select bit in IPU_CONF register selects which CSI is routed to either the VDIC or the IC, and is independent of whether the IC is set to receive from a CSI or the VDIC. Sugested by Marek Vasut. - ov5640: propagate error codes from all i2c register accesses. Sugested by Sakari Ailus . - ov5640: drop the entity stream count check in ov5640_s_stream(). Sugested by Sakari Ailus . - ov5640: Fix manual exposure control. The manual exposure setting (in line periods) cannot exceed the max exposure value in registers {0x380E, 0x380F} + {0x350C,0x350D}, however the max eposure value was not being calcualted correctly. - ov5640: the video mode register tables require auto gain/exposure be disabled before programming the register set. However auto gain/exp was being disabled by direct register write. This caused the auto gain/exp control values to be inconsistent with the actual hardware setting. Fixed by going through v4l2-ctrl when disabling auto gain/exp. - ov5640: converted virtual channel macro to a module parameter, default to channel 0. - ov5640: override the v4l2-ctl lock to use the ov5640 subdev driver's lock. Sugested by Sakari Ailus . - ov5640: switch to unit-less V4L2_CID_EXPOSURE. Using V4L2_CID_EXPOSURE_ABSOLUTE will require converting from 100-usec units to line periods and vice-versa. Sugested by Sakari Ailus and Pavel Machek . - ov5640: drop dangling regulator_bulk_disable() from probe/remove. Sugested by Sakari Ailus . - FIM: move input capture channel selection out of the device-tree and make this a V4L2 control. In order to support attaching a FIM to prpencvf, the FIM cannot have any device-tree configuration, because prpencvf has no device node. The FIM now is completely configurable via its V4L2 controls. - FIM: drop imx_media_fim_set_power(), and move the input capture channel request to imx_media_fim_set_stream(). This allows to drop csi_s_power() as well, since the latter only called imx_media_fim_set_power(). - FIM: add a spinlock to protect the frame_interval_monitor() from the setting of new control values. The frame_interval_monitor() is called from interrupt context so a spinlock must be used. - Updated to version 8 video-mux patchset from Philipp Zabel . Marek Vasut (1): media: imx: Drop warning upon multiple S_STREAM disable calls Philipp Zabel (8): dt-bindings: Add bindings for video-multiplexer device ARM: dts: imx6qdl: add multiplexer controls ARM: dts: imx6qdl: Add video multiplexers, mipi_csi, and their connections add mux and video interface bridge entity functions platform: add video-multiplexer subdevice driver media: imx: csi: increase burst size for YUV formats media: imx: csi: add frame skipping support media: imx: csi: add sink selection rectangles Russell King (3): media: imx: csi: add support for bayer formats media: imx: csi: add frame size/interval enumeration media: imx: capture: add frame sizes/interval enumeration Steve Longerbeam (22): [media] dt-bindings: Add bindings for i.MX media driver [media] dt/bindings: Add bindings for OV5640 ARM: dts: imx6qdl: Add compatible, clocks, irqs to MIPI CSI-2 node ARM: dts: imx6qdl: add capture-subsystem device ARM: dts: imx6qdl-sabrelite: remove erratum ERR006687 workaround ARM: dts: imx6-sabrelite: add OV5642 and OV5640 camera sensors ARM: dts: imx6-sabresd: add OV5642 and OV5640 camera sensors ARM: dts: imx6-sabreauto: create i2cmux for i2c3 ARM: dts: imx6-sabreauto: add reset-gpios property for max7310_b ARM: dts: imx6-sabreauto: add pinctrl for gpt input capture ARM: dts: imx6-sabreauto: add the ADV7180 video decoder [media] add Omnivision OV5640 sensor driver media: Add userspace header file for i.MX media: Add i.MX media core driver media: imx: Add a TODO file media: imx: Add Capture Device Interface media: imx: Add CSI subdev driver media: imx: Add VDIC subdev driver media: imx: Add IC subdev drivers media: imx: Add MIPI CSI-2 Receiver subdev driver media: imx: set and propagate default field, colorimetry ARM: imx_v6_v7_defconfig: Enable staging video4linux drivers .../devicetree/bindings/media/i2c/ov5640.txt | 45 + Documentation/devicetree/bindings/media/imx.txt| 47 + .../devicetree/bindings/media/video-mux.txt| 60 + Documentation/media/uapi/mediactl/media-types.rst | 21 + Documentation/media/v4l-drivers/imx.rst| 614 + arch/arm/boot/dts/imx6dl-sabrelite.dts |5 + arch/arm/boot/dts/imx6dl-sabresd.dts |