On Wed, Oct 17, 2018 at 4:37 PM Steve Longerbeam <slongerbeam@xxxxxxxxx> wrote: > > > On 10/17/18 4:05 PM, Tim Harvey wrote: > > On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam <slongerbeam@xxxxxxxxx> wrote: > >> Hi Tim, > >> > >> On 10/17/18 1:38 PM, Tim Harvey wrote: > >> > >> On Mon, Jun 4, 2018 at 1:58 AM Krzysztof Hałasa <khalasa@xxxxxxx> wrote: > >> > >> I've just tested the PAL setup: in currect situation (v4.17 + Steve's > >> fix-csi-interlaced.2 + "media: adv7180: fix field type" + a small cheap > >> PAL camera) the following produces bottom-first interlaced frames: > >> > >> media-ctl -r -l '"adv7180 2-0020":0->"ipu2_csi1_mux":1[1], > >> "ipu2_csi1_mux":2->"ipu2_csi1":0[1], > >> "ipu2_csi1":2->"ipu2_csi1 capture":0[1]' > >> > >> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x576 field:alternate]" > >> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x576]" > >> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x576 field:interlaced]" > >> > >> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1_mux":1 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1_mux":2 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1":0 [fmt:UYVY2X8/720x576 field:alternate] > >> "ipu2_csi1":2 [fmt:AYUV32/720x576 field:interlaced] > >> > >> I think it would be great if these changes make their way upstream. > >> The details could be refined then. > >> > >> Krzysztof / Steve / Philipp, > >> > >> I jumped back onto IMX6 video capture from the adv7180 the other day > >> trying to help out a customer that's using mainline and found things > >> are still not working right. Where is all of this at these days? > >> > >> If I use v4.19 with Steves 'imx-media: Fixes for interlaced capture' > >> v3 series (https://patchwork.kernel.org/cover/10626499/) I > >> rolling/split (un-synchronized) video using: > >> > >> # Setup links > >> media-ctl -r > >> media-ctl -l '"adv7180 2-0020":0 -> "ipu2_csi1_mux":1[1]' > >> media-ctl -l '"ipu2_csi1_mux":2 -> "ipu2_csi1":0[1]' > >> media-ctl -l '"ipu2_csi1":1 -> "ipu2_ic_prp":0[1]' > >> media-ctl -l '"ipu2_ic_prp":2 -> "ipu2_ic_prpvf":0[1]' > >> media-ctl -l '"ipu2_ic_prpvf":1 -> "ipu2_ic_prpvf capture":0[1]' > >> # Configure pads > >> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480]" > >> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_csi1':1 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_ic_prp':2 [fmt:UYVY2X8/720x480 field:interlaced]" > >> media-ctl -V "'ipu2_ic_prpvf':1 [fmt:UYVY2X8/720x480 field:none]" > >> # stream JPEG/RTP/UDP > >> gst-launch-1.0 v4l2src device=/dev/video3 ! video/x-raw,format=UYVY ! > >> jpegenc ! rtpjpegpay ! udpsink host=$SERVER port=$PORT > >> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Device > >> '/dev/video3' does not support progressive interlacing > >> > >> I'm doing the above on a Gateworks GW5404 IMXQ which has a tda1997x > >> HDMI receiver sensor and an adv7180 Analog CVBS sensor - media graph > >> is here: http://dev.gateworks.com/docs/linux/media/imx6q-gw54xx-media.png > >> > >> Are there other patches I need or different field formats above with > >> 4.19? Do any of the other kernels work without patchsets that you know > >> of between 4.16 and 4.19? > >> > >> > >> First, the v3 series is out of date. Please apply the latest v5 posting > >> of that series. See the imx.rst doc regarding field type negotiation, > >> all pads starting at ipu2_csi1:1 should be 'seq-bt' or 'seq-tb' until the > >> capture device, which should be set to 'interlaced' to enable IDMAC > >> interweave. The ADV7180 now correctly sets its field type to alternate, > >> which imx-media-csi.c translates to seq-tb or seq-bt at its output pad. > >> > >> See the SabreAuto examples in the doc. > >> > >> For the rolling/split image problem, try the attached somewhat hackish patch. > >> There used to be code in imx-media-csi.c that searched for the backend sensor > >> and queries via .g_skip_frames whether the sensor produces bad frames at first > >> stream-on. But there was push-back on that, so the attached is another > >> approach that doesn't require searching for a backend sensor. > > Steve, > > > > Thanks - I hadn't noticed the updated series. I've built it on top of > > linux-media/master and tested with: > > > > - Testing linux-media/master + your v5 now: > > > > # Use simple interweaving > > media-ctl -r > > # Setup links > > media-ctl -l '"adv7180 2-0020":0 -> "ipu2_csi1_mux":1[1]' > > media-ctl -l '"ipu2_csi1_mux":2 -> "ipu2_csi1":0[1]' > > media-ctl -l '"ipu2_csi1":2 -> "ipu2_csi1 capture":0[1]' > > # Configure pads > > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]" > > media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]" > > media-ctl -V "'ipu2_csi1':1 [fmt:AYUV32/720x480]" > > # Configure ipu_csi capture interface (/dev/video7) > > v4l2-ctl -d7 --set-fmt-video=field=interlaced_bt > > # Stream JPEG/RTP/UDP > > gst-launch-1.0 v4l2src device=/dev/video7 ! video/x-raw,format=UYVY ! > > jpegenc ! rtpjpegpay ! udpsink host=$SERVER port=5000 > > ^^^^^^ gives me ERROR: from element > > /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Device '/dev/video7' does > > not support progressive interlacing > > > > I'm assuming this is because the format is still 'interlaced' - not > > sure how to stream this from GStreamer? > > > I don't know what v4l2src plugin is trying to say by "progressive > interlacing" - > that's meaningless, the video is either progressive or interlaced, not both. > > But what is probably meant is v4l2src is trying to set field type at > /dev/video7 > to 'none', and complains that it can't. That's a bug in v4l2src, it > should accept > 'interlaced'. > > > I'm not getting this error in the version of v42lsrc I have been testing > with, it > must be something added recently. Haven't looked at the v4l2src git log > yet. > Steve, Your right the above was not working using GStreamer 1.14.1 from an Ubuntu Bionic rootfs but works fine Using GStreamer 1.8.3 on an Ubuntu Xenial rootfs. I'll ask about this with the GStreamer folk. > > > > > # Use VDIC motion compensated de-interlace > > # Setup links > > media-ctl -r > > media-ctl -l "'adv7180 2-0020':0 -> 'ipu2_csi1_mux':1[1]" > > media-ctl -l "'ipu2_csi1_mux':2 -> 'ipu2_csi1':0[1]" > > media-ctl -l "'ipu2_csi1':1 -> 'ipu2_vdic':0[1]" > > media-ctl -l "'ipu2_vdic':2 -> 'ipu2_ic_prp':0[1]" > > media-ctl -l "'ipu2_ic_prp':2 -> 'ipu2_ic_prpvf':0[1]" > > media-ctl -l "'ipu2_ic_prpvf':1 -> 'ipu2_ic_prpvf capture':0[1]" > > # Configure pads > > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-tb]" > > media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]" > > media-ctl -V "'ipu2_csi1':1 [fmt:AYUV32/720x480]" > > media-ctl -V "'ipu2_vdic':2 [fmt:AYUV32/720x480 field:none]" > > media-ctl -V "'ipu2_ic_prp':2 [fmt:AYUV32/720x480 field:none]" > > media-ctl -V "'ipu2_ic_prpvf':1 [fmt:AYUV32/720x480 field:none]" > > # Stream JPEG/RTP/UDP > > gst-launch-1.0 v4l2src device=/dev/video3 ! video/x-raw,format=UYVY ! > > jpegenc ! rtpjpegpay ! udpsink host=$SERVER port=5000 > > ^^^^^ streams but still shows sync issues > > > > But once I add your patch it does resolve this (with the 10 frame > > skip). Strangely I don't recall having to do this way back when your > > imx-media driver was still going through revisions? > > > That's because the bad frame skipping existed in prior versions, > I removed it due to negative feedback at > > bf3cfaa712 ("media: staging/imx: get CSI bus type from nearest upstream > entity") > Thanks for that explanation. I tested v4.15 (before the use of g_skip_frames was removed) and it still shows the same invalid sync with adv7180 issue because adv7180 doesn't implement g_skip_frames. Adding it with a quick patch to skip just 2 frames works fine: diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c index 6fb818a..0285627 100644 --- a/drivers/media/i2c/adv7180.c +++ b/drivers/media/i2c/adv7180.c @@ -187,6 +187,9 @@ #define ADV7180_DEFAULT_CSI_I2C_ADDR 0x44 #define ADV7180_DEFAULT_VPP_I2C_ADDR 0x42 +/* Initial number of frames to skip to avoid possible garbage */ +#define ADV7180_NUM_OF_SKIP_FRAMES 2 + #define V4L2_CID_ADV_FAST_SWITCH (V4L2_CID_USER_ADV7180_BASE + 0x00) struct adv7180_state; @@ -759,6 +762,13 @@ static int adv7180_g_mbus_config(struct v4l2_subdev *sd, return 0; } +static int adv7180_get_skip_frames(struct v4l2_subdev *sd, u32 *frames) +{ + *frames = ADV7180_NUM_OF_SKIP_FRAMES; + + return 0; +} + static int adv7180_g_pixelaspect(struct v4l2_subdev *sd, struct v4l2_fract *aspect) { struct adv7180_state *state = to_state(sd); @@ -838,10 +848,15 @@ static const struct v4l2_subdev_pad_ops adv7180_pad_ops = { .get_fmt = adv7180_get_pad_format, }; +static const struct v4l2_subdev_sensor_ops adv7180_sensor_ops = { + .g_skip_frames = adv7180_get_skip_frames, +}; + static const struct v4l2_subdev_ops adv7180_ops = { .core = &adv7180_core_ops, .video = &adv7180_video_ops, .pad = &adv7180_pad_ops, + .sensor = &adv7180_sensor_ops, }; static irqreturn_t adv7180_irq(int irq, void *devid) So I still don't quite know what I was testing in the past that didn't show this adv7180 sync issue. I'm curious how Krzysztof dealt with it in his recent testing with v4.19. Its very likely that I was getting around the issue by using your FIM solution which perhaps is the right solution here as well. FIM also has the added benefit of resolving the issue (on the capture side not the sensor side) of sync breaking during loss of signal during streaming which I have had to resolve for people switching inputs during streaming. Where was the negative feedback with the use of g_skip_frames? It looks to me like v4l2-subdev.h describes g_skip_frames specifically for this purpose as its described in the header as "number of frames to skip at stream start. This is needed for buggy sensors that generate faulty frames when they are turned on.". Perhaps use of g_skip_frames should be added back to media/imx/imx-media-csi.c as well as an implementation of it added to adv7180? Or perhaps FIM resolves both of these issue? > > > > > I haven't enabled FIM yet and don't recall how to do so from > > userspace now that its using V4L2 CID's. > > > It's easy! From ipu2_csi1_capture device /dev/video7 from above pipeline: > > v4l2-ctl -d7 --set-ctrl=fim_enable=1 > > > > Is there a way to set > > V4L2_CID_IMX_FIM_NUM_SKIP, V4L2_CID_IMX_FIM_ICAP_CHANNEL and > > V4L2_CID_IMX_FIM_ICAP_EDGE from userspace to test? > > v4l2-ctl -d7 --set-ctrl=fim_num_skip=N > that doesn't work (using v4l2-ctl from git master on Oct 4) - you must be using a patched version of v4l2-ctl perhaps? I wasn't sure if there was a v4l2-ctl usage that let you define CID's by number instead of by name? > etc. > > But input capture is not operational yet. I posted the patch to imx6 > clocksource driver a while ago, no replies. I can forward that patch to > you, it will require some machinations elsewhere in imx-media driver to > enable icap support though IIRC. Note also that input capture also requires > hardware support: the ADV7180 FIELD output pin must be routed to one of > the imx6 > input capture pads. Right, you explain clock capture pretty well in Documentation/media/v4l-drivers/imx.rst. I don't have VSYNC going to an input capture channel so I have to rely on FIM working via EOF interrupt. Tim
