i.MX6 IPU CSI analog video input on Ventana

i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Hi,

I'm using analog PAL video in on GW53xx/54xx boards (through ADV7180
chip and 8-bit parallel CSI input, with (presumably) BT.656).
I'm trying to upgrade from e.g. Linux 4.2 + Steve's older MX6 camera
driver (which works fine) to v.4.16 with the recently merged driver.

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:interlaced]'
media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x576 field:interlaced]'
media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x576 field:interlaced]'

It seems there are issues, though:

First, I can't find a way to change to PAL standard. *s_std() doesn't
propagate from "ipu2_csi1 capture" through "ipu2_csi1_mux" to adv7180.

For now I have just changed the default:
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1320,7 +1321,7 @@ static int adv7180_probe(struct i2c_client *client,
 
     state->irq = client->irq;
     mutex_init(&state->mutex);
-    state->curr_norm = V4L2_STD_NTSC;
+    state->curr_norm = V4L2_STD_PAL;
     if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
         state->powered = true;
     else


Second, the image format information I'm getting out of "ipu2_csi1
capture" device is:

open("/dev/video6")
ioctl(VIDIOC_S_FMT, {V4L2_BUF_TYPE_VIDEO_CAPTURE,
	fmt.pix={704x576, pixelformat=NV12, V4L2_FIELD_INTERLACED} =>
	fmt.pix={720x576, pixelformat=NV12, V4L2_FIELD_INTERLACED,
        bytesperline=720, sizeimage=622080,
	colorspace=V4L2_COLORSPACE_SMPTE170M}})

Now, the resulting image obtained via QBUF/DQBUF doesn't seem to be
a single interlaced frame (like it was with older drivers). Actually,
I'm getting the two fields, encoded with NV12 and concatenated
together (I think it's V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT).

What's wrong?
Is it possible to get a real V4L2_FIELD_INTERLACED frame, so it can be
passed straight to the CODA H.264 encoder?
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hello Krzysztof,


On 05/10/2018 01:19 AM, Krzysztof Hałasa wrote:
> Hi,
>
> I'm using analog PAL video in on GW53xx/54xx boards (through ADV7180
> chip and 8-bit parallel CSI input, with (presumably) BT.656).
> I'm trying to upgrade from e.g. Linux 4.2 + Steve's older MX6 camera
> driver (which works fine) to v.4.16 with the recently merged driver.
>
> 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:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x576 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x576 field:interlaced]'
>
> It seems there are issues, though:
>
> First, I can't find a way to change to PAL standard. *s_std() doesn't
> propagate from "ipu2_csi1 capture" through "ipu2_csi1_mux" to adv7180.

Right. That's a current drawback, other mc drivers have this issue
too. One option, besides changing the default below, is to make
VIDIOC_QUERYSTD, VIDIOC_G_STD, and VIDIOC_S_STD ioctls
available for use via v4l2 subdevice node, as in:

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c 
b/drivers/media/v4l2-core/v4l2-subdev.c
index 43fefa7..fedc347 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -195,6 +195,15 @@ static long subdev_do_ioctl(struct file *file, 
unsigned int cmd, void *arg)
      case VIDIOC_QUERYMENU:
          return v4l2_querymenu(vfh->ctrl_handler, arg);

+    case VIDIOC_QUERYSTD:
+        return v4l2_subdev_call(sd, video, querystd, arg);
+
+    case VIDIOC_G_STD:
+        return v4l2_subdev_call(sd, video, g_std, arg);
+
+    case VIDIOC_S_STD:
+        return v4l2_subdev_call(sd, video, s_std, *(v4l2_std_id *)arg);
+
      case VIDIOC_G_CTRL:
          return v4l2_g_ctrl(vfh->ctrl_handler, arg);


>
> For now I have just changed the default:
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -1320,7 +1321,7 @@ static int adv7180_probe(struct i2c_client *client,
>   
>       state->irq = client->irq;
>       mutex_init(&state->mutex);
> -    state->curr_norm = V4L2_STD_NTSC;
> +    state->curr_norm = V4L2_STD_PAL;
>       if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
>           state->powered = true;
>       else
>
>
> Second, the image format information I'm getting out of "ipu2_csi1
> capture" device is:
>
> open("/dev/video6")
> ioctl(VIDIOC_S_FMT, {V4L2_BUF_TYPE_VIDEO_CAPTURE,
> 	fmt.pix={704x576, pixelformat=NV12, V4L2_FIELD_INTERLACED} =>
> 	fmt.pix={720x576, pixelformat=NV12, V4L2_FIELD_INTERLACED,
>          bytesperline=720, sizeimage=622080,
> 	colorspace=V4L2_COLORSPACE_SMPTE170M}})
>
> Now, the resulting image obtained via QBUF/DQBUF doesn't seem to be
> a single interlaced frame (like it was with older drivers). Actually,
> I'm getting the two fields, encoded with NV12 and concatenated
> together (I think it's V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT).
>
> What's wrong?

Set field type at /dev/video6 to NONE. That will enable IDMAC
interweaving of the top and bottom fields.

Steve

> Is it possible to get a real V4L2_FIELD_INTERLACED frame, so it can be
> passed straight to the CODA H.264 encoder?

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

>> Second, the image format information I'm getting out of "ipu2_csi1
>> capture" device is:
>>
>> open("/dev/video6")
>> ioctl(VIDIOC_S_FMT, {V4L2_BUF_TYPE_VIDEO_CAPTURE,
>> 	fmt.pix={704x576, pixelformat=NV12, V4L2_FIELD_INTERLACED} =>
>> 	fmt.pix={720x576, pixelformat=NV12, V4L2_FIELD_INTERLACED,
>>          bytesperline=720, sizeimage=622080,
>> 	colorspace=V4L2_COLORSPACE_SMPTE170M}})
>>
>> Now, the resulting image obtained via QBUF/DQBUF doesn't seem to be
>> a single interlaced frame (like it was with older drivers). Actually,
>> I'm getting the two fields, encoded with NV12 and concatenated
>> together (I think it's V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT).
>>
>> What's wrong?
>
> Set field type at /dev/video6 to NONE. That will enable IDMAC
> interweaving of the top and bottom fields.

Such as this?
"adv7180 2-0020":0
                [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":1
                [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":2
                [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":0
                [fmt:UYVY2X8/720x576 field:interlaced
                 crop.bounds:(0,0)/720x576
                 crop:(0,0)/720x576
                 compose.bounds:(0,0)/720x576
                 compose:(0,0)/720x576]
"ipu2_csi1":2
                [fmt:AYUV32/720x576 field:none]

There is something wrong - the resulting image is out of (vertical)
sync, it seems the time it takes to receive a frame is a bit longer than
the normal 40 ms. I can also set field to NONE on "ipu2_csi1_mux":[12]
but it doesn't sync, either. Only with everything set to INTERLACED, the
frame is synchronized (actually, it starts unsynchronized, but slowly
scrolls down the screen and eventually "catches sync").
With the old drivers nothing like this happens: the image is "instantly"
synchronized and it's a single interlaced frame, not the two halves
concatenated.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 05/10/2018 10:37 PM, Krzysztof Hałasa wrote:
> Steve Longerbeam <slongerbeam@gmail.com> writes:
>
>>> Second, the image format information I'm getting out of "ipu2_csi1
>>> capture" device is:
>>>
>>> open("/dev/video6")
>>> ioctl(VIDIOC_S_FMT, {V4L2_BUF_TYPE_VIDEO_CAPTURE,
>>> 	fmt.pix={704x576, pixelformat=NV12, V4L2_FIELD_INTERLACED} =>
>>> 	fmt.pix={720x576, pixelformat=NV12, V4L2_FIELD_INTERLACED,
>>>           bytesperline=720, sizeimage=622080,
>>> 	colorspace=V4L2_COLORSPACE_SMPTE170M}})
>>>
>>> Now, the resulting image obtained via QBUF/DQBUF doesn't seem to be
>>> a single interlaced frame (like it was with older drivers). Actually,
>>> I'm getting the two fields, encoded with NV12 and concatenated
>>> together (I think it's V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT).
>>>
>>> What's wrong?
>> Set field type at /dev/video6 to NONE. That will enable IDMAC
>> interweaving of the top and bottom fields.
> Such as this?
> "adv7180 2-0020":0
>                  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1
>                  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":2
>                  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":0
>                  [fmt:UYVY2X8/720x576 field:interlaced
>                   crop.bounds:(0,0)/720x576
>                   crop:(0,0)/720x576
>                   compose.bounds:(0,0)/720x576
>                   compose:(0,0)/720x576]
> "ipu2_csi1":2
>                  [fmt:AYUV32/720x576 field:none]


Yes, that looks fine.

> There is something wrong - the resulting image is out of (vertical)
> sync,

Yes, the CSI on i.MX6 does not deal well with unstable bt.656 sync codes,
which results in vertical sync issues (scrolling or split images). The 
ADV7180
will often shift the sync codes around in various situations (initial 
power on,
see below, also when there is an interruption of the input analog CVBS
signal).

There is a frame interval monitor in the imx-media driver that can catch 
these
unstable sync code events and send an v4l2 event to userspace, userspace can
then issue stream off->on which usually corrects the vertical sync.

See https://linuxtv.org/downloads/v4l-dvb-apis/v4l-drivers/imx.html, section
15.9 for more info on the vertical sync issues in i.MX6 CSI and how to setup
the FIM to correct them.

One other thing I've noticed is that the ADV7180 can send unstable bt.656
sync codes after initial power on. Try adding a ~10 frame time delay in
adv7180_set_power(), so that the imx CSI won't see these frames and
get tripped up at stream on.


>   it seems the time it takes to receive a frame is a bit longer than
> the normal 40 ms. I can also set field to NONE on "ipu2_csi1_mux":[12]
> but it doesn't sync, either. Only with everything set to INTERLACED, the
> frame is synchronized (actually, it starts unsynchronized, but slowly
> scrolls down the screen and eventually "catches sync").
> With the old drivers nothing like this happens: the image is "instantly"
> synchronized and it's a single interlaced frame, not the two halves
> concatenated.

The old driver, IIRC, would also catch the unstable sync codes via
a FIM, but would internally restart IPU capture hardware without
the involvement of userspace, effectively this was an internal
stream on/off. In imx-media, this must be done via userspace when
it catches the V4L2_EVENT_IMX_FRAME_INTERVAL_ERROR event.

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

> Yes, the CSI on i.MX6 does not deal well with unstable bt.656 sync codes,
> which results in vertical sync issues (scrolling or split images). The
> ADV7180
> will often shift the sync codes around in various situations (initial
> power on,
> see below, also when there is an interruption of the input analog CVBS
> signal).

I'm not convinced it's the sync code issue. I've compared the key
registers (both 4.2 + your old driver vs 4.16) and this is what I got:

"adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":2      [fmt:AYUV32/720x576 field:none]

There is H sync but no V sync. The encoding is wrong (I'm using NV12 but
what I get from /dev/video* isn't NV12).

IPU2_CSI1 registers are:
                0        4        8 C 10       14       18       1C
2a38000: 04000A20 023F02CF 023F02CF 0  0 00040030 00000000 00FF0000
vs the old driver:
         04000A30 027002CF 023F02CF 0  0 01040596 000D07DF 00FF0000

0: CSI1 Sensor Configuration (IPUx_CSI1_SENS_CONF)
The new driver uses progressive mode while the old one - interlaced
mode.

4: CSI1 Sense Frame Size Register (IPUx_CSI1_SENS_FRM_SIZE)
The new driver uses 575 lines in place of 624 (this probably needs to be
checked with the ADV7180 docs, though the old version works fine).

14, 18: CSI1 CCIR Code Register 1 and 2 (IPUx_CSI1_CCIR_CODE_[12])
The new driver doesn't use "Error detection and correction" and it seems
the codes are set for progressive mode. I think this can't work.


With:
"adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:none]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:none]
"ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:none]
"ipu2_csi1":2      [fmt:AYUV32/720x576 field:none]

Still, V sync but no H sync. The Y/colors are good, except that
there are two consecutive images on the screen.
2a38000: 04000A20 023F02CF 023F02CF 0  0 00040030 00000000 00FF0000
CSI set to progressive again. Setting the registers manually (SENS_CONF
and SAV/EAV codes) makes the image stabilize, though there are still two
images (split in the middle). Apparently something is simply appending
the two field images, instead of merging them properly.


With:
"adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":2      [fmt:AYUV32/720x576 field:interlaced]

2a38000: 04000A30 027002CF 023F02CF 0  0 01040596 000D07DF 00FF0000
the CSI is set for interlaced mode, and there are two stable images
(both fields concatenated).


The first case again (all except ipu2_csi1 set to interlaced). I've
manually set the CSI registers and now the image is synchronized and
stable (one complete frame this time). The problem is it's not NV12
(nor YUV420), the colors are all green and the Y lines comes in pairs -
valid then invalid (probably color) and so on.


Could it be a DTS problem? I'm using imx6q-gw53xx.dtb file,
the 8-bit ADV7180 (40 pin version) is connected to the IPU2 CSI1 DATA,
EIM_EB3 = HSYNC, EIM_A16 = PIXCLK and EIM_D29 = VSYNC. HSYNC and VSYNC
aren't currently used, though.

I Guess I have to compare all IPU registers.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Tim Harvey]

On Fri, May 18, 2018 at 10:28 AM, Krzysztof Hałasa <khalasa@piap.pl> wrote:
> Steve Longerbeam <slongerbeam@gmail.com> writes:
>
>> Yes, the CSI on i.MX6 does not deal well with unstable bt.656 sync codes,
>> which results in vertical sync issues (scrolling or split images). The
>> ADV7180
>> will often shift the sync codes around in various situations (initial
>> power on,
>> see below, also when there is an interruption of the input analog CVBS
>> signal).
>
> I'm not convinced it's the sync code issue. I've compared the key
> registers (both 4.2 + your old driver vs 4.16) and this is what I got:
>
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":2      [fmt:AYUV32/720x576 field:none]
>
> There is H sync but no V sync. The encoding is wrong (I'm using NV12 but
> what I get from /dev/video* isn't NV12).
>
> IPU2_CSI1 registers are:
>                 0        4        8 C 10       14       18       1C
> 2a38000: 04000A20 023F02CF 023F02CF 0  0 00040030 00000000 00FF0000
> vs the old driver:
>          04000A30 027002CF 023F02CF 0  0 01040596 000D07DF 00FF0000
>
> 0: CSI1 Sensor Configuration (IPUx_CSI1_SENS_CONF)
> The new driver uses progressive mode while the old one - interlaced
> mode.
>
> 4: CSI1 Sense Frame Size Register (IPUx_CSI1_SENS_FRM_SIZE)
> The new driver uses 575 lines in place of 624 (this probably needs to be
> checked with the ADV7180 docs, though the old version works fine).
>
> 14, 18: CSI1 CCIR Code Register 1 and 2 (IPUx_CSI1_CCIR_CODE_[12])
> The new driver doesn't use "Error detection and correction" and it seems
> the codes are set for progressive mode. I think this can't work.
>
>
> With:
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:none]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:none]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:none]
> "ipu2_csi1":2      [fmt:AYUV32/720x576 field:none]
>
> Still, V sync but no H sync. The Y/colors are good, except that
> there are two consecutive images on the screen.
> 2a38000: 04000A20 023F02CF 023F02CF 0  0 00040030 00000000 00FF0000
> CSI set to progressive again. Setting the registers manually (SENS_CONF
> and SAV/EAV codes) makes the image stabilize, though there are still two
> images (split in the middle). Apparently something is simply appending
> the two field images, instead of merging them properly.
>
>
> With:
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":2      [fmt:AYUV32/720x576 field:interlaced]
>
> 2a38000: 04000A30 027002CF 023F02CF 0  0 01040596 000D07DF 00FF0000
> the CSI is set for interlaced mode, and there are two stable images
> (both fields concatenated).
>
>
> The first case again (all except ipu2_csi1 set to interlaced). I've
> manually set the CSI registers and now the image is synchronized and
> stable (one complete frame this time). The problem is it's not NV12
> (nor YUV420), the colors are all green and the Y lines comes in pairs -
> valid then invalid (probably color) and so on.
>
>
> Could it be a DTS problem? I'm using imx6q-gw53xx.dtb file,
> the 8-bit ADV7180 (40 pin version) is connected to the IPU2 CSI1 DATA,
> EIM_EB3 = HSYNC, EIM_A16 = PIXCLK and EIM_D29 = VSYNC. HSYNC and VSYNC
> aren't currently used, though.
>
> I Guess I have to compare all IPU registers.

Krzysztof,

What version of kernel are you using and what specific board model do
you have (full board model and/or serial number so I know if you've
got an IMX6DL or an IMX6Q) and have you modified the device-tree? I
tested the adv7180 a couple of months ago but I don't know if I hit
your specific combination. I was using NTSC but if your not getting
VSYNC then yes I would like to make sure the pinmux is correct for
your situation.

Thanks,

Tim

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Tim,

Tim Harvey <tharvey@gateworks.com> writes:

> What version of kernel are you using and what specific board model do
> you have (full board model and/or serial number so I know if you've
> got an IMX6DL or an IMX6Q) and have you modified the device-tree? I
> tested the adv7180 a couple of months ago but I don't know if I hit
> your specific combination. I was using NTSC but if your not getting
> VSYNC then yes I would like to make sure the pinmux is correct for
> your situation.

At the moment I'm using 4.16.0 and this particular board IDs itself as
GW5304-D2, the CPU is i.MX6Q. I mostly use GW5300s and GW5400s (and
others in smaller numbers) and they work fine with Steve's older driver
(the one with a special ADV7180 module).

I'm using the official device tree (v4.16 in this case) with just the
LDB (LVDS display) portion removed.

I guess I can test it with a NTSC camera.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Tested with NTSC camera, it's the same as with PAL.
The only case when IPU2_CSI1_SENS_CONF register is set to interlaced
mode (PRCTL=3, CCIR interlaced mode (BT.656)) is when all parts of the
pipeline are set to interlaced:

"adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
"ipu2_csi1":2      [fmt:AYUV32/720x576 field:interlaced]

The image is stable and in sync, the "only" problem is that I get two
concatenated field images (in one V4L2 frame) instead of a normal
interlaced frame (all lines in order - 0, 1, 2, 3, 4 etc).
IOW I get V4L2_FIELD_ALTERNATE, V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT
(the data format, I don't mean the pixel format.field) while I need to
get V4L2_FIELD_INTERLACED, V4L2_FIELD_INTERLACED_TB or _BT.


If I set "ipu2_csi1":2 to field:none, the IPU2_CSI1_SENS_CONF is set to
progressive mode (PRCTL=2). It's the last element of the pipeline I can
configure, it's connected straight to "ipu2_csi1 capture" aka
/dev/videoX. I think CSI can't work with interlaced camera (and ADV7180)
when set to progressive, can it?


I wonder... perhaps to get an interlaced frame I need to route the data
through VDIC (ipu2_vdic, the deinterlacer)?
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Tim Harvey]

On Mon, May 21, 2018 at 1:09 AM, Krzysztof Hałasa <khalasa@piap.pl> wrote:
> Tested with NTSC camera, it's the same as with PAL.
> The only case when IPU2_CSI1_SENS_CONF register is set to interlaced
> mode (PRCTL=3, CCIR interlaced mode (BT.656)) is when all parts of the
> pipeline are set to interlaced:
>
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":2      [fmt:AYUV32/720x576 field:interlaced]
>
> The image is stable and in sync, the "only" problem is that I get two
> concatenated field images (in one V4L2 frame) instead of a normal
> interlaced frame (all lines in order - 0, 1, 2, 3, 4 etc).
> IOW I get V4L2_FIELD_ALTERNATE, V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT
> (the data format, I don't mean the pixel format.field) while I need to
> get V4L2_FIELD_INTERLACED, V4L2_FIELD_INTERLACED_TB or _BT.
>
>
> If I set "ipu2_csi1":2 to field:none, the IPU2_CSI1_SENS_CONF is set to
> progressive mode (PRCTL=2). It's the last element of the pipeline I can
> configure, it's connected straight to "ipu2_csi1 capture" aka
> /dev/videoX. I think CSI can't work with interlaced camera (and ADV7180)
> when set to progressive, can it?
>
>
> I wonder... perhaps to get an interlaced frame I need to route the data
> through VDIC (ipu2_vdic, the deinterlacer)?

Krzysztof,

Right, your doing a raw capture where you get both fields in one
buffer and I'm not clear what to do with that.

Here's what I've used on a GW54xx with IMX6Q and an adv7180 for NTSC.

using VDIC to deinterlace:
# adv7180 -> vdic -> ic_prpvf -> /dev/video3
# VDIC will de-interlace using motion compensation
media-ctl -r # reset all links
# 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":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]"
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_vdic':2 [fmt:UYVY2X8/720x480 field:interlaced]"
media-ctl -V "'ipu2_ic_prp':2 [fmt:UYVY2X8/720x480 field:none]"
media-ctl -V "'ipu2_ic_prpvf':1 [fmt:UYVY2X8/720x480 field:none]"
# streaming can now begin on /dev/video3
v4l2-ctl -d3 --set-fmt-video=width=720,height=480,pixelformat=UYVY
v4l2-ctl -d3 --set-ctrl=deinterlacing_mode=3 # set max motion
compensation (default)
#^^^^ this is the default so could be skipped; also its the only value
allowed when capturing direct from CSI
v4l2-ctl -d3 --stream-mmap --stream-to=/x.raw --stream-count=1 # capture 1 frame
convert -size 720x480 -depth 16 uyvy:/x.raw /var/www/html/frame.png #
and convert
# or stream jpeg's via gst
gst-launch-1.0 v4l2src device=/dev/video3 ! "video/x-raw,format=UYVY"
! jpegenc ! queue ! avimux name=mux ! udpsink host=172.24.20.19
port=5000

or de-interlace via IDMAC:
# PRPVF will do simple IDMAC line interweaving for de-interlacing,
since VDIC is not involved in the pipeline, but it will only enable
this in the IDMAC if it sees interlaced input at prpvf
media-ctl -r # reset all links
# 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":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]"
# streaming can now begin on /dev/video3
v4l2-ctl -d3 --set-fmt-video=width=720,height=480,pixelformat=UYVY
v4l2-ctl -d3 --stream-mmap --stream-to=/x.raw --stream-count=1 # capture
gst-launch-1.0 v4l2src device=/dev/video3 ! "video/x-raw,format=UYVY"
! jpegenc ! queue ! avimux name=mux ! udpsink host=172.24.20.19
port=5000

or the following for non deinterlaced:
# adv7180 -> ic_prp -> ic_prpenc -> /dev/video2
media-ctl -r # reset all links
# 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":1 -> "ipu2_ic_prp":0[1]'
media-ctl -l '"ipu2_ic_prp":1 -> "ipu2_ic_prpenc":0[1]'
media-ctl -l '"ipu2_ic_prpenc":1 -> "ipu2_ic_prpenc 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':1 [fmt:UYVY2X8/720x480 field:interlaced]"
media-ctl -V "'ipu2_ic_prpenc':1 [fmt:UYVY2X8/720x480 field:none]"
# streaming can now begin on /dev/video2
v4l2-ctl -d2 --set-fmt-video=width=720,height=480,pixelformat=UYVY
v4l2-ctl -d2 --stream-mmap --stream-to=/x.raw --stream-count=1 # capture
gst-launch-1.0 v4l2src device=/dev/video2 ! "video/x-raw,format=UYVY"
! jpegenc ! queue ! avimux name=mux ! udpsink host=172.24.20.19
port=5000

One of these days I intend to document all of this on our Gateworks
wiki. Its complex as heck with all the board and CPU variants. I wish
there was a tool that would auto-connect the entitites as the inputs
and outputs change depending on CPU variant but I'm not aware of
anything that does that yet. I'm also not very clear on all the
possibilities - Steve is the expert on that.

Tim

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof, I've been on vacation, just returned today. I will
find the time this week to attempt to reproduce your results on
a SabreAuto quad with the adv7180.

Btw, if you just need to capture an interlaced frame (lines 0,1,2,...)
without motion compensation, there is no need to use the VDIC
path. Capturing directly from ipu2_csi1 should work, I've tested
this many times on a SabreAuto. But I will try to reproduce your
results.

Steve


On 05/21/2018 01:09 AM, Krzysztof Hałasa wrote:
> Tested with NTSC camera, it's the same as with PAL.
> The only case when IPU2_CSI1_SENS_CONF register is set to interlaced
> mode (PRCTL=3, CCIR interlaced mode (BT.656)) is when all parts of the
> pipeline are set to interlaced:
>
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x576 field:interlaced]
> "ipu2_csi1":2      [fmt:AYUV32/720x576 field:interlaced]
>
> The image is stable and in sync, the "only" problem is that I get two
> concatenated field images (in one V4L2 frame) instead of a normal
> interlaced frame (all lines in order - 0, 1, 2, 3, 4 etc).
> IOW I get V4L2_FIELD_ALTERNATE, V4L2_FIELD_SEQ_TB or V4L2_FIELD_SEQ_BT
> (the data format, I don't mean the pixel format.field) while I need to
> get V4L2_FIELD_INTERLACED, V4L2_FIELD_INTERLACED_TB or _BT.
>
>
> If I set "ipu2_csi1":2 to field:none, the IPU2_CSI1_SENS_CONF is set to
> progressive mode (PRCTL=2). It's the last element of the pipeline I can
> configure, it's connected straight to "ipu2_csi1 capture" aka
> /dev/videoX. I think CSI can't work with interlaced camera (and ADV7180)
> when set to progressive, can it?
>
>
> I wonder... perhaps to get an interlaced frame I need to route the data
> through VDIC (ipu2_vdic, the deinterlacer)?

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Hi,

Steve Longerbeam <slongerbeam@gmail.com> writes:

> Hi Krzysztof, I've been on vacation, just returned today. I will
> find the time this week to attempt to reproduce your results on
> a SabreAuto quad with the adv7180.

Great. Please let me know if I can assist you somehow.

> Btw, if you just need to capture an interlaced frame (lines 0,1,2,...)
> without motion compensation, there is no need to use the VDIC
> path. Capturing directly from ipu2_csi1 should work, I've tested
> this many times on a SabreAuto. But I will try to reproduce your
> results.

That's what I was thinking. Thanks a lot.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Hi,

I've experimented with the ADV7180 a bit and this is what I found.

First, I'm using (with a NTSC camera but I guess PAL won't be much
different):
media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:interlaced]'

IOW I set all of the parts to interlaced mode. If i set the last element
to "none", the CSI is not set for interlaced input, and nothing works at
the low level.

This requires a quick temporary hack:
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -474,8 +474,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 
 	ipu_smfc_set_burstsize(priv->smfc, burst_size);
 
-	if (image.pix.field == V4L2_FIELD_NONE &&
-	    V4L2_FIELD_HAS_BOTH(infmt->field))
+	if (1 || (image.pix.field == V4L2_FIELD_NONE &&
+		  V4L2_FIELD_HAS_BOTH(infmt->field)))
 		ipu_cpmem_interlaced_scan(priv->idmac_ch,
 					  image.pix.bytesperline);
 

I.e., I need to set CPMEM to interlaced mode when I operate CSI in
interlaced mode. The original code is a bit unclear to me in fact.

The following is required as well. Now the question is why we can't skip
writing those odd UV rows. Anyway, with these 2 changes, I get a stable
NTSC (and probably PAL) interlaced video stream.

The manual says: Reduce Double Read or Writes (RDRW):
This bit is relevant for YUV4:2:0 formats. For write channels:
U and V components are not written to odd rows.

How could it be so? With YUV420, are they normally written?
OTOH it seems that not only UV is broken with this bit set.
Y is broken as well.

--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -413,14 +413,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
 		passthrough_bits = 16;
 		break;
 	case V4L2_PIX_FMT_YUV420:
 	case V4L2_PIX_FMT_NV12:
 		burst_size = (image.pix.width & 0x3f) ?
 			     ((image.pix.width & 0x1f) ?
 			      ((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
 		passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
 		passthrough_bits = 16;
-		/* Skip writing U and V components to odd rows */
-		ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
 		break;
 	case V4L2_PIX_FMT_YUYV:
 	case V4L2_PIX_FMT_UYVY:
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof,


On 05/24/2018 08:56 AM, Krzysztof Hałasa wrote:
> Hi,
>
> I've experimented with the ADV7180 a bit and this is what I found.
>
> First, I'm using (with a NTSC camera but I guess PAL won't be much
> different):
> media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:interlaced]'
>
> IOW I set all of the parts to interlaced mode. If i set the last element
> to "none", the CSI is not set for interlaced input, and nothing works at
> the low level.

This is what I don't understand. By setting pad ipu2_csi1:2 to
"none", the if statement below should be true (sink pad field
is "interlaced" and the capture field is propagated from ipu2_csi1:2
field so it is "none", thus ipu_cpmem_interlaced_scan() will be called.

And yes you are correct, ipu_cpmem_interlaced_scan() must be
called to enable IDMAC interweave, which is what you want.

>
> This requires a quick temporary hack:
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -474,8 +474,8 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   
>   	ipu_smfc_set_burstsize(priv->smfc, burst_size);
>   
> -	if (image.pix.field == V4L2_FIELD_NONE &&
> -	    V4L2_FIELD_HAS_BOTH(infmt->field))
> +	if (1 || (image.pix.field == V4L2_FIELD_NONE &&
> +		  V4L2_FIELD_HAS_BOTH(infmt->field)))
>   		ipu_cpmem_interlaced_scan(priv->idmac_ch,
>   					  image.pix.bytesperline);
>   
>
> I.e., I need to set CPMEM to interlaced mode when I operate CSI in
> interlaced mode. The original code is a bit unclear to me in fact.

No the code above is not unclear at all. The if statement is saying
that if the user wants progressive output (V4L2_FIELD_NONE), and
the input contains fields, then turn on interweave in the IDMAC
channel.

This might be a good time to bring up the fact that the ADV7180 driver 
is wrong
to set output to "interlaced". The ADV7180 does not transmit top lines 
interlaced
with bottom lines. It transmits all top lines followed by all bottom 
lines (or
vice-versa), i.e. it should be either V4L2_FIELD_SEQ_TB or 
V4L2_FIELD_SEQ_BT.
It can also be set to V4L2_FIELD_ALTERNATE, and then it is left up to 
downstream
elements to determine field order (TB or BT).

I've previously sent a patch to fix this at 
https://patchwork.linuxtv.org/patch/36193/
but it got lost. Niklas has said he will pick this up again.

>
> The following is required as well. Now the question is why we can't skip
> writing those odd UV rows. Anyway, with these 2 changes, I get a stable
> NTSC (and probably PAL) interlaced video stream.
>
> The manual says: Reduce Double Read or Writes (RDRW):
> This bit is relevant for YUV4:2:0 formats. For write channels:
> U and V components are not written to odd rows.
>
> How could it be so? With YUV420, are they normally written?

Well, given that this bit exists, and assuming I understand it correctly 
(1),
I guess the U and V components for odd rows normally are placed on the
AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
the U and V components are the same for odd and even rows.

In other words for writing 4:2:0 to memory, this bit should _always_ be set.

(1) OTOH I don't really understand what this bit is trying to say.
Whether this bit is set or not, the data in memory is correct
for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
(decimated by 2 in width and height), followed by Y plane of 1/4
size.

So I assume it is saying that the IPU normally places U/V components
on the AXI bus for odd rows, that are identical to the even row values.
IOW somehow those identical odd rows are dropped before writing to
the U/V planes in memory.

Philipp please chime in if you have something to add here.

Steve

> OTOH it seems that not only UV is broken with this bit set.
> Y is broken as well.
>
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -413,14 +413,12 @@ static int csi_idmac_setup_channel(struct csi_priv *priv)
>   		passthrough_bits = 16;
>   		break;
>   	case V4L2_PIX_FMT_YUV420:
>   	case V4L2_PIX_FMT_NV12:
>   		burst_size = (image.pix.width & 0x3f) ?
>   			     ((image.pix.width & 0x1f) ?
>   			      ((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
>   		passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
>   		passthrough_bits = 16;
> -		/* Skip writing U and V components to odd rows */
> -		ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
>   		break;
>   	case V4L2_PIX_FMT_YUYV:
>   	case V4L2_PIX_FMT_UYVY:

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof,

Sorry I did find a bug. Please try this patch:

--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -631,7 +631,6 @@ static int csi_setup(struct csi_priv *priv)
  {
         struct v4l2_mbus_framefmt *infmt, *outfmt;
         struct v4l2_mbus_config mbus_cfg;
-       struct v4l2_mbus_framefmt if_fmt;

         infmt = &priv->format_mbus[CSI_SINK_PAD];
         outfmt = &priv->format_mbus[priv->active_output_pad];
@@ -642,20 +641,13 @@ static int csi_setup(struct csi_priv *priv)
                 priv->upstream_ep.bus.mipi_csi2.flags :
                 priv->upstream_ep.bus.parallel.flags;

-       /*
-        * we need to pass input frame to CSI interface, but
-        * with translated field type from output format
-        */
-       if_fmt = *infmt;
-       if_fmt.field = outfmt->field;
-
         ipu_csi_set_window(priv->csi, &priv->crop);

         ipu_csi_set_downsize(priv->csi,
                              priv->crop.width == 2 * priv->compose.width,
                              priv->crop.height == 2 * 
priv->compose.height);

-       ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+       ipu_csi_init_interface(priv->csi, &mbus_cfg, infmt);

         ipu_csi_set_dest(priv->csi, priv->dest);


(the removed code was meant to deal with field type at sink pad being
"alternate", which ipu_csi_init_interface() doesn't currently recognize, but
that should be dealt with in IPUv3 driver).

With that you should be able to set pad ipu2_csi1:2 to field type 
"none", e.g.
set pipeline to:

media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:none]'

With the above patch, capture from ipu1_csi0:2 is fixed for me on SabreAuto.

You may also want to try adding a ~500 msec delay after adv7180 power on
as I explained earlier:

--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -503,6 +503,9 @@ static int adv7180_set_power(struct adv7180_state 
*state, bool on)
                 }
         }

+       if (on)
+               msleep(500);
+
         return 0;
  }


Steve


On 05/24/2018 11:12 AM, Steve Longerbeam wrote:
> Hi Krzysztof,
>
>
> On 05/24/2018 08:56 AM, Krzysztof Hałasa wrote:
>> Hi,
>>
>> I've experimented with the ADV7180 a bit and this is what I found.
>>
>> First, I'm using (with a NTSC camera but I guess PAL won't be much
>> different):
>> media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
>> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
>> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
>> media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
>> media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:interlaced]'
>>
>> IOW I set all of the parts to interlaced mode. If i set the last element
>> to "none", the CSI is not set for interlaced input, and nothing works at
>> the low level.
>
> This is what I don't understand. By setting pad ipu2_csi1:2 to
> "none", the if statement below should be true (sink pad field
> is "interlaced" and the capture field is propagated from ipu2_csi1:2
> field so it is "none", thus ipu_cpmem_interlaced_scan() will be called.
>
> And yes you are correct, ipu_cpmem_interlaced_scan() must be
> called to enable IDMAC interweave, which is what you want.
>
>>
>> This requires a quick temporary hack:
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -474,8 +474,8 @@ static int csi_idmac_setup_channel(struct 
>> csi_priv *priv)
>>         ipu_smfc_set_burstsize(priv->smfc, burst_size);
>>   -    if (image.pix.field == V4L2_FIELD_NONE &&
>> -        V4L2_FIELD_HAS_BOTH(infmt->field))
>> +    if (1 || (image.pix.field == V4L2_FIELD_NONE &&
>> +          V4L2_FIELD_HAS_BOTH(infmt->field)))
>>           ipu_cpmem_interlaced_scan(priv->idmac_ch,
>>                         image.pix.bytesperline);
>>
>> I.e., I need to set CPMEM to interlaced mode when I operate CSI in
>> interlaced mode. The original code is a bit unclear to me in fact.
>
> No the code above is not unclear at all. The if statement is saying
> that if the user wants progressive output (V4L2_FIELD_NONE), and
> the input contains fields, then turn on interweave in the IDMAC
> channel.
>
> This might be a good time to bring up the fact that the ADV7180 driver 
> is wrong
> to set output to "interlaced". The ADV7180 does not transmit top lines 
> interlaced
> with bottom lines. It transmits all top lines followed by all bottom 
> lines (or
> vice-versa), i.e. it should be either V4L2_FIELD_SEQ_TB or 
> V4L2_FIELD_SEQ_BT.
> It can also be set to V4L2_FIELD_ALTERNATE, and then it is left up to 
> downstream
> elements to determine field order (TB or BT).
>
> I've previously sent a patch to fix this at 
> https://patchwork.linuxtv.org/patch/36193/
> but it got lost. Niklas has said he will pick this up again.
>
>>
>> The following is required as well. Now the question is why we can't skip
>> writing those odd UV rows. Anyway, with these 2 changes, I get a stable
>> NTSC (and probably PAL) interlaced video stream.
>>
>> The manual says: Reduce Double Read or Writes (RDRW):
>> This bit is relevant for YUV4:2:0 formats. For write channels:
>> U and V components are not written to odd rows.
>>
>> How could it be so? With YUV420, are they normally written?
>
> Well, given that this bit exists, and assuming I understand it 
> correctly (1),
> I guess the U and V components for odd rows normally are placed on the
> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> the U and V components are the same for odd and even rows.
>
> In other words for writing 4:2:0 to memory, this bit should _always_ 
> be set.
>
> (1) OTOH I don't really understand what this bit is trying to say.
> Whether this bit is set or not, the data in memory is correct
> for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
> (decimated by 2 in width and height), followed by Y plane of 1/4
> size.
>
> So I assume it is saying that the IPU normally places U/V components
> on the AXI bus for odd rows, that are identical to the even row values.
> IOW somehow those identical odd rows are dropped before writing to
> the U/V planes in memory.
>
> Philipp please chime in if you have something to add here.
>
> Steve
>
>> OTOH it seems that not only UV is broken with this bit set.
>> Y is broken as well.
>>
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -413,14 +413,12 @@ static int csi_idmac_setup_channel(struct 
>> csi_priv *priv)
>>           passthrough_bits = 16;
>>           break;
>>       case V4L2_PIX_FMT_YUV420:
>>       case V4L2_PIX_FMT_NV12:
>>           burst_size = (image.pix.width & 0x3f) ?
>>                    ((image.pix.width & 0x1f) ?
>>                     ((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
>>           passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
>>           passthrough_bits = 16;
>> -        /* Skip writing U and V components to odd rows */
>> -        ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
>>           break;
>>       case V4L2_PIX_FMT_YUYV:
>>       case V4L2_PIX_FMT_UYVY:
>

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof, Philipp,

And I can confirm that capturing planar 4:2:0 (YU12, YV12, or NV12),
is broken because of the call to ipu_cpmem_skip_odd_chroma_rows().
YU12 or NV12 images look correct again when commenting out that
call. Commits

14330d7f08 ("media: imx: csi: enable double write reduction")
b54a5c2dc8 ("media: imx: prpencvf: enable double write reduction")

should be reverted for now, until the behavior of this bit is better 
understood.

Steve

On 05/24/2018 01:48 PM, Steve Longerbeam wrote:
> Hi Krzysztof,
>
> Sorry I did find a bug. Please try this patch:
>
> --- a/drivers/staging/media/imx/imx-media-csi.c
> +++ b/drivers/staging/media/imx/imx-media-csi.c
> @@ -631,7 +631,6 @@ static int csi_setup(struct csi_priv *priv)
>  {
>         struct v4l2_mbus_framefmt *infmt, *outfmt;
>         struct v4l2_mbus_config mbus_cfg;
> -       struct v4l2_mbus_framefmt if_fmt;
>
>         infmt = &priv->format_mbus[CSI_SINK_PAD];
>         outfmt = &priv->format_mbus[priv->active_output_pad];
> @@ -642,20 +641,13 @@ static int csi_setup(struct csi_priv *priv)
>                 priv->upstream_ep.bus.mipi_csi2.flags :
>                 priv->upstream_ep.bus.parallel.flags;
>
> -       /*
> -        * we need to pass input frame to CSI interface, but
> -        * with translated field type from output format
> -        */
> -       if_fmt = *infmt;
> -       if_fmt.field = outfmt->field;
> -
>         ipu_csi_set_window(priv->csi, &priv->crop);
>
>         ipu_csi_set_downsize(priv->csi,
>                              priv->crop.width == 2 * priv->compose.width,
>                              priv->crop.height == 2 * 
> priv->compose.height);
>
> -       ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
> +       ipu_csi_init_interface(priv->csi, &mbus_cfg, infmt);
>
>         ipu_csi_set_dest(priv->csi, priv->dest);
>
>
> (the removed code was meant to deal with field type at sink pad being
> "alternate", which ipu_csi_init_interface() doesn't currently 
> recognize, but
> that should be dealt with in IPUv3 driver).
>
> With that you should be able to set pad ipu2_csi1:2 to field type 
> "none", e.g.
> set pipeline to:
>
> media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:none]'
>
> With the above patch, capture from ipu1_csi0:2 is fixed for me on 
> SabreAuto.
>
> You may also want to try adding a ~500 msec delay after adv7180 power on
> as I explained earlier:
>
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -503,6 +503,9 @@ static int adv7180_set_power(struct adv7180_state 
> *state, bool on)
>                 }
>         }
>
> +       if (on)
> +               msleep(500);
> +
>         return 0;
>  }
>
>
> Steve
>
>
> On 05/24/2018 11:12 AM, Steve Longerbeam wrote:
>> Hi Krzysztof,
>>
>>
>> On 05/24/2018 08:56 AM, Krzysztof Hałasa wrote:
>>> Hi,
>>>
>>> I've experimented with the ADV7180 a bit and this is what I found.
>>>
>>> First, I'm using (with a NTSC camera but I guess PAL won't be much
>>> different):
>>> media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
>>> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
>>> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
>>> media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
>>> media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:interlaced]'
>>>
>>> IOW I set all of the parts to interlaced mode. If i set the last 
>>> element
>>> to "none", the CSI is not set for interlaced input, and nothing 
>>> works at
>>> the low level.
>>
>> This is what I don't understand. By setting pad ipu2_csi1:2 to
>> "none", the if statement below should be true (sink pad field
>> is "interlaced" and the capture field is propagated from ipu2_csi1:2
>> field so it is "none", thus ipu_cpmem_interlaced_scan() will be called.
>>
>> And yes you are correct, ipu_cpmem_interlaced_scan() must be
>> called to enable IDMAC interweave, which is what you want.
>>
>>>
>>> This requires a quick temporary hack:
>>> --- a/drivers/staging/media/imx/imx-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>>> @@ -474,8 +474,8 @@ static int csi_idmac_setup_channel(struct 
>>> csi_priv *priv)
>>>         ipu_smfc_set_burstsize(priv->smfc, burst_size);
>>>   -    if (image.pix.field == V4L2_FIELD_NONE &&
>>> -        V4L2_FIELD_HAS_BOTH(infmt->field))
>>> +    if (1 || (image.pix.field == V4L2_FIELD_NONE &&
>>> +          V4L2_FIELD_HAS_BOTH(infmt->field)))
>>>           ipu_cpmem_interlaced_scan(priv->idmac_ch,
>>>                         image.pix.bytesperline);
>>>
>>> I.e., I need to set CPMEM to interlaced mode when I operate CSI in
>>> interlaced mode. The original code is a bit unclear to me in fact.
>>
>> No the code above is not unclear at all. The if statement is saying
>> that if the user wants progressive output (V4L2_FIELD_NONE), and
>> the input contains fields, then turn on interweave in the IDMAC
>> channel.
>>
>> This might be a good time to bring up the fact that the ADV7180 
>> driver is wrong
>> to set output to "interlaced". The ADV7180 does not transmit top 
>> lines interlaced
>> with bottom lines. It transmits all top lines followed by all bottom 
>> lines (or
>> vice-versa), i.e. it should be either V4L2_FIELD_SEQ_TB or 
>> V4L2_FIELD_SEQ_BT.
>> It can also be set to V4L2_FIELD_ALTERNATE, and then it is left up to 
>> downstream
>> elements to determine field order (TB or BT).
>>
>> I've previously sent a patch to fix this at 
>> https://patchwork.linuxtv.org/patch/36193/
>> but it got lost. Niklas has said he will pick this up again.
>>
>>>
>>> The following is required as well. Now the question is why we can't 
>>> skip
>>> writing those odd UV rows. Anyway, with these 2 changes, I get a stable
>>> NTSC (and probably PAL) interlaced video stream.
>>>
>>> The manual says: Reduce Double Read or Writes (RDRW):
>>> This bit is relevant for YUV4:2:0 formats. For write channels:
>>> U and V components are not written to odd rows.
>>>
>>> How could it be so? With YUV420, are they normally written?
>>
>> Well, given that this bit exists, and assuming I understand it 
>> correctly (1),
>> I guess the U and V components for odd rows normally are placed on the
>> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
>> the U and V components are the same for odd and even rows.
>>
>> In other words for writing 4:2:0 to memory, this bit should _always_ 
>> be set.
>>
>> (1) OTOH I don't really understand what this bit is trying to say.
>> Whether this bit is set or not, the data in memory is correct
>> for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
>> (decimated by 2 in width and height), followed by Y plane of 1/4
>> size.
>>
>> So I assume it is saying that the IPU normally places U/V components
>> on the AXI bus for odd rows, that are identical to the even row values.
>> IOW somehow those identical odd rows are dropped before writing to
>> the U/V planes in memory.
>>
>> Philipp please chime in if you have something to add here.
>>
>> Steve
>>
>>> OTOH it seems that not only UV is broken with this bit set.
>>> Y is broken as well.
>>>
>>> --- a/drivers/staging/media/imx/imx-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>>> @@ -413,14 +413,12 @@ static int csi_idmac_setup_channel(struct 
>>> csi_priv *priv)
>>>           passthrough_bits = 16;
>>>           break;
>>>       case V4L2_PIX_FMT_YUV420:
>>>       case V4L2_PIX_FMT_NV12:
>>>           burst_size = (image.pix.width & 0x3f) ?
>>>                    ((image.pix.width & 0x1f) ?
>>>                     ((image.pix.width & 0xf) ? 8 : 16) : 32) : 64;
>>>           passthrough = is_parallel_16bit_bus(&priv->upstream_ep);
>>>           passthrough_bits = 16;
>>> -        /* Skip writing U and V components to odd rows */
>>> -        ipu_cpmem_skip_odd_chroma_rows(priv->idmac_ch);
>>>           break;
>>>       case V4L2_PIX_FMT_YUYV:
>>>       case V4L2_PIX_FMT_UYVY:
>>
>

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

Hi Steve,

On Thu, 2018-05-24 at 14:33 -0700, Steve Longerbeam wrote:
> Hi Krzysztof, Philipp,
> 
> And I can confirm that capturing planar 4:2:0 (YU12, YV12, or NV12),
> is broken because of the call to ipu_cpmem_skip_odd_chroma_rows().
> YU12 or NV12 images look correct again when commenting out that
> call. Commits
> 
> 14330d7f08 ("media: imx: csi: enable double write reduction")
> b54a5c2dc8 ("media: imx: prpencvf: enable double write reduction")
> 
> should be reverted for now, until the behavior of this bit is better 
> understood.

I think that is a bit radical. I am not aware of any problems with non-
interlaced formats. Could we just disable them when the interlaced_scan
bit is set?

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

> Sorry I did find a bug. Please try this patch:

Ok, your patch fixes the first problem (sets the CSI interlaced mode
on input when field = NOE is requested on output). Posting in full since
your mail came somehow mangled with UTF-8.

--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -629,7 +629,6 @@ static int csi_setup(struct csi_priv *priv)
 {
 	struct v4l2_mbus_framefmt *infmt, *outfmt;
 	struct v4l2_mbus_config mbus_cfg;
-	struct v4l2_mbus_framefmt if_fmt;
 
 	infmt = &priv->format_mbus[CSI_SINK_PAD];
 	outfmt = &priv->format_mbus[priv->active_output_pad];
@@ -640,20 +639,13 @@ static int csi_setup(struct csi_priv *priv)
 		priv->upstream_ep.bus.mipi_csi2.flags :
 		priv->upstream_ep.bus.parallel.flags;
 
-	/*
-	 * we need to pass input frame to CSI interface, but
-	 * with translated field type from output format
-	 */
-	if_fmt = *infmt;
-	if_fmt.field = outfmt->field;
-
 	ipu_csi_set_window(priv->csi, &priv->crop);
 
 	ipu_csi_set_downsize(priv->csi,
 			     priv->crop.width == 2 * priv->compose.width,
 			     priv->crop.height == 2 * priv->compose.height);
 
-	ipu_csi_init_interface(priv->csi, &mbus_cfg, &if_fmt);
+	ipu_csi_init_interface(priv->csi, &mbus_cfg, infmt);
 
 	ipu_csi_set_dest(priv->csi, priv->dest);
 

> (the removed code was meant to deal with field type at sink pad being
> "alternate", which ipu_csi_init_interface() doesn't currently recognize, but
> that should be dealt with in IPUv3 driver).

I see.

> With that you should be able to set pad ipu2_csi1:2 to field type
> "none", e.g.
> set pipeline to:
>
> media-ctl -V '"adv7180 2-0020":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":1[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1_mux":2[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":0[fmt:UYVY2X8 720x480 field:interlaced]'
> media-ctl -V '"ipu2_csi1":2[fmt:UYVY2X8 720x480 field:none]'
>
> With the above patch, capture from ipu1_csi0:2 is fixed for me on
> SabreAuto.

Right, it also works fine for me on Ventana GW5300 (with
ipu_cpmem_skip_odd_chroma_rows() removed as well, of course).

> You may also want to try adding a ~500 msec delay after adv7180 power on
> as I explained earlier:

Ok. In fact I don't have a sync problem even without it, the rolling
image always eventually syncs. Maybe I'll investigate the data stream
(from ADV7180 to CSI) and see what's on. I't a bit complicated since
what I have is just an oscilloscope.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
[...]
> > The following is required as well. Now the question is why we can't skip
> > writing those odd UV rows. Anyway, with these 2 changes, I get a stable
> > NTSC (and probably PAL) interlaced video stream.
> > 
> > The manual says: Reduce Double Read or Writes (RDRW):
> > This bit is relevant for YUV4:2:0 formats. For write channels:
> > U and V components are not written to odd rows.
> > 
> > How could it be so? With YUV420, are they normally written?
> 
> Well, given that this bit exists, and assuming I understand it correctly 
> (1),
> I guess the U and V components for odd rows normally are placed on the
> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> the U and V components are the same for odd and even rows.
> 
> In other words for writing 4:2:0 to memory, this bit should _always_ be set.
> 
> (1) OTOH I don't really understand what this bit is trying to say.
> Whether this bit is set or not, the data in memory is correct
> for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
> (decimated by 2 in width and height), followed by Y plane of 1/4
> size.
> 
> So I assume it is saying that the IPU normally places U/V components
> on the AXI bus for odd rows, that are identical to the even row values.

Whether they are identical depends on the input format.

The IDMAC always gets fed AYUV32 from the CSI or IC.
If the CSI captures YUV 4:2:x, odd and even lines will have the same
chroma values. But if the CSI captures YUV 4:4:4 (or RGB, fed through
the IC), we can have AYUV32 input with different chroma values on even
and odd lines.
In that case the IPU just writes the even chroma line and then
overwrites it with the odd line, unless the double write reduction bit
is set.

> IOW somehow those identical odd rows are dropped before writing to
> the U/V planes in memory.

potentially identical.

> Philipp please chime in if you have something to add here.

I suppose the bit could be used to choose to write the chroma values of
odd instead of even lines for 4:4:4 inputs, at the cost of increased
memory bandwidth usage.

> Steve
> 
> > OTOH it seems that not only UV is broken with this bit set.
> > Y is broken as well.

Maybe scanline interlave and double write reduction can't be used at the
same time?

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Philipp Zabel <p.zabel@pengutronix.de> writes:

> Maybe scanline interlave and double write reduction can't be used at the
> same time?

Well, if it works in non-interlaced modes - it may be the case.

Perhaps the data reduction is done before the field merge step. This
would make it incompatible: in interlaced mode we need all color data
from a field (we could potentially remove all color info from the other
field, or use some average).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof, Philipp,


On 05/25/2018 12:18 AM, Krzysztof Hałasa wrote:
> Philipp Zabel <p.zabel@pengutronix.de> writes:
>
>> Maybe scanline interlave and double write reduction can't be used at the
>> same time?
> Well, if it works in non-interlaced modes - it may be the case.
>
> Perhaps the data reduction is done before the field merge step.

Yeah, that might explain the incompatibility. The IDMAC top/bottom
line merging needs all the lines present. It won't have them if the
IDMAC has previously skipped the odd chroma lines. Or maybe I'm
over-simplifying.

In any case as I said they are proved to be incompatible. I am
preparing a patch-set with these fixes.

Krzysztof, in the meantime the patches are available in my
media-tree fork, for testing on the Ventana GW5300:

git@github.com:slongerbeam/mediatree.git, branch 'fix-csi-interlaced'

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Hi Steve,

Steve Longerbeam <slongerbeam@gmail.com> writes:

> Krzysztof, in the meantime the patches are available in my
> media-tree fork, for testing on the Ventana GW5300:
>
> git@github.com:slongerbeam/mediatree.git, branch 'fix-csi-interlaced'

I assume fix-csi-interlaced.2 is a newer version, isn't it?

I merged it and I think I can't set the correct config:
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/720x480 field:seq-bt]"
media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced]"

produces:
"adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:interlaced]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:interlaced]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:interlaced]
"ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:interlaced]
"ipu2_csi1":2      [fmt:AYUV32/720x480  field:interlaced]

Do I need to patch ADV7180 for field type "sequential"?

It seems setting seq-bt on ADV7180 sets "interlaced" on ADV -> MUX input
-> MUX output. Setting "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]" sets
interlaced on all elements of the pipeline. The effect is a pair of
fields, not an interlaced frame.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof,


On 05/29/2018 12:26 AM, Krzysztof Hałasa wrote:
> Hi Steve,
>
> Steve Longerbeam <slongerbeam@gmail.com> writes:
>
>> Krzysztof, in the meantime the patches are available in my
>> media-tree fork, for testing on the Ventana GW5300:
>>
>> git@github.com:slongerbeam/mediatree.git, branch 'fix-csi-interlaced'
> I assume fix-csi-interlaced.2 is a newer version, isn't it?
>
> I merged it and I think I can't set the correct config:
> 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/720x480 field:seq-bt]"
> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced]"
>
> produces:
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:interlaced]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:interlaced]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:interlaced]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:interlaced]
> "ipu2_csi1":2      [fmt:AYUV32/720x480  field:interlaced]
>
> Do I need to patch ADV7180 for field type "sequential"?

Yes, you'll need to patch adv7180.c to select either
'seq-bt/tb' or 'alternate'. The current version will override
any attempt to set field to anything other than 'interlaced'.
This is in anticipation of getting a patch merged for adv7180
that fixes this.

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

> Yes, you'll need to patch adv7180.c to select either
> 'seq-bt/tb' or 'alternate'. The current version will override
> any attempt to set field to anything other than 'interlaced'.
> This is in anticipation of getting a patch merged for adv7180
> that fixes this.

Right. I've applied the patch from your adv718x-v6 branch (just the
"media: adv7180: fix field type" patch) and now it works.

Also, I have changed "seq-bt" to "alternate" (in the examples in
Documentation/media/v4l-drivers/imx.rst) - the data stream from ADV7180
to CSI consists of separate fields which can then be merged into frames
in any order requested by the user (e.g. in accordance with "digital PAL
/ NTSC" requirements).

The following:
media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced]"
now produces:

"adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced-bt]

and it works correctly.

The only issue is that I can't:
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced-tb]"
(it remains fixed in -bt mode since NTSC is the default). I think we may
set TB/BT by default (depending on CSI input geometry or TV standard),
but it should be possible for the user to explicitly request the field
order on CSI output (I can make a patch I guess).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Krzysztof,


On 05/30/2018 01:53 AM, Krzysztof Hałasa wrote:
> Steve Longerbeam <slongerbeam@gmail.com> writes:
>
>> Yes, you'll need to patch adv7180.c to select either
>> 'seq-bt/tb' or 'alternate'. The current version will override
>> any attempt to set field to anything other than 'interlaced'.
>> This is in anticipation of getting a patch merged for adv7180
>> that fixes this.
> Right. I've applied the patch from your adv718x-v6 branch (just the
> "media: adv7180: fix field type" patch) and now it works.
>
> Also, I have changed "seq-bt" to "alternate" (in the examples in
> Documentation/media/v4l-drivers/imx.rst) - the data stream from ADV7180
> to CSI consists of separate fields which can then be merged into frames
> in any order requested by the user (e.g. in accordance with "digital PAL
> / NTSC" requirements).
>
> The following:
> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced]"
> now produces:
>
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced-bt]
>
> and it works correctly.
>
> The only issue is that I can't:
> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced-tb]"
> (it remains fixed in -bt mode since NTSC is the default). I think we may
> set TB/BT by default (depending on CSI input geometry or TV standard),

Yes, that's what I've implemented. If the user requests an interlaced
field type ('interlaced', 'interlaced-bt', 'interlaced-tb'), but the field
order is not correct given the input height (480=NTSC, 576=PAL),
then the request field type is overridden with the correct field order.

> but it should be possible for the user to explicitly request the field
> order on CSI output (I can make a patch I guess).

If you think that is the correct behavior, I will remove the override
code. I suppose it makes sense to allow user to select field order even
if that order does not make sense given the input standard. I'm fine
either way, Philipp what is your opinion? I'll go with the popular vote :)

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

>> but it should be possible for the user to explicitly request the field
>> order on CSI output (I can make a patch I guess).
>
> If you think that is the correct behavior, I will remove the override
> code. I suppose it makes sense to allow user to select field order even
> if that order does not make sense given the input standard. I'm fine
> either way, Philipp what is your opinion? I'll go with the popular vote :)

I think it should be up to the user.
Actually, PAL and NTSC aren't valid names in the digital world. Their
meaning ends in the ADV7180 (or equivalent). I don't know if PAL and/or
NTSC specify the field order in the analog frame (meaningful when
someone hooks a camera with progressive sensor and analog, interlaced
output), but the digital YUV422 from ADV to CSI isn't NTSC/PAL anymore.
It's just WxH @ framerate + possible interlacing, sequential fields,
top-bottom or otherwise, etc. The analog standard names could be used
here but just as defaults.

If we were strict (and we don't want to force it), then we should set
NTSC/PAL thing on ADV7180 input, 720x480@29.97i (or 720x576@50i, or
704x... etc) on the input parts of the CSI/IPU (where there are no video
frames yet), and 720x480@29.97i B-T or T-B (or default, or separate
fields - whatever suits the user) on the output from CSI.

I remember the reversed field order was sometimes needed - for example,
PAL DV (the casette camcorder thing) produced B-T frames (same as NTSC),
and to avoid (slight) additional quality loss one had to process it
(up to e.g. .MP4, DVD, and then to HDMI, SCART etc) as B-T.
It wasn't a problem in otherwise-PAL-centric environment.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 05/30/2018 11:46 AM, Krzysztof Hałasa wrote:
> Steve Longerbeam <slongerbeam@gmail.com> writes:
>
>>> but it should be possible for the user to explicitly request the field
>>> order on CSI output (I can make a patch I guess).
>> If you think that is the correct behavior, I will remove the override
>> code. I suppose it makes sense to allow user to select field order even
>> if that order does not make sense given the input standard. I'm fine
>> either way, Philipp what is your opinion? I'll go with the popular vote :)
> I think it should be up to the user.
> Actually, PAL and NTSC aren't valid names in the digital world. Their
> meaning ends in the ADV7180 (or equivalent). I don't know if PAL and/or
> NTSC specify the field order in the analog frame (meaningful when
> someone hooks a camera with progressive sensor and analog, interlaced
> output), but the digital YUV422 from ADV to CSI isn't NTSC/PAL anymore.
> It's just WxH @ framerate + possible interlacing, sequential fields,
> top-bottom or otherwise, etc. The analog standard names could be used
> here but just as defaults.
>
> If we were strict (and we don't want to force it), then we should set
> NTSC/PAL thing on ADV7180 input, 720x480@29.97i (or 720x576@50i, or
> 704x... etc) on the input parts of the CSI/IPU (where there are no video
> frames yet), and 720x480@29.97i B-T or T-B (or default, or separate
> fields - whatever suits the user) on the output from CSI.
>
> I remember the reversed field order was sometimes needed - for example,
> PAL DV (the casette camcorder thing) produced B-T frames (same as NTSC),
> and to avoid (slight) additional quality loss one had to process it
> (up to e.g. .MP4, DVD, and then to HDMI, SCART etc) as B-T.
> It wasn't a problem in otherwise-PAL-centric environment.

I tend to agree, I've found conflicting info out there regarding
PAL vs. NTSC field order. And I've never liked having to guess
at input analog standard based on input # lines. I will go ahead
and remove the field order override code.

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Wed, May 30, 2018 at 01:56:34PM -0700, Steve Longerbeam wrote:
> 
> 
> On 05/30/2018 11:46 AM, Krzysztof Hałasa wrote:
> > Steve Longerbeam <slongerbeam@gmail.com> writes:
> > 
> > > > but it should be possible for the user to explicitly request the field
> > > > order on CSI output (I can make a patch I guess).
> > > If you think that is the correct behavior, I will remove the override
> > > code. I suppose it makes sense to allow user to select field order even
> > > if that order does not make sense given the input standard. I'm fine
> > > either way, Philipp what is your opinion? I'll go with the popular vote :)
> > I think it should be up to the user.
> > Actually, PAL and NTSC aren't valid names in the digital world. Their
> > meaning ends in the ADV7180 (or equivalent). I don't know if PAL and/or
> > NTSC specify the field order in the analog frame (meaningful when
> > someone hooks a camera with progressive sensor and analog, interlaced
> > output), but the digital YUV422 from ADV to CSI isn't NTSC/PAL anymore.
> > It's just WxH @ framerate + possible interlacing, sequential fields,
> > top-bottom or otherwise, etc. The analog standard names could be used
> > here but just as defaults.
> > 
> > If we were strict (and we don't want to force it), then we should set
> > NTSC/PAL thing on ADV7180 input, 720x480@29.97i (or 720x576@50i, or
> > 704x... etc) on the input parts of the CSI/IPU (where there are no video
> > frames yet), and 720x480@29.97i B-T or T-B (or default, or separate
> > fields - whatever suits the user) on the output from CSI.
> > 
> > I remember the reversed field order was sometimes needed - for example,
> > PAL DV (the casette camcorder thing) produced B-T frames (same as NTSC),
> > and to avoid (slight) additional quality loss one had to process it
> > (up to e.g. .MP4, DVD, and then to HDMI, SCART etc) as B-T.
> > It wasn't a problem in otherwise-PAL-centric environment.
> 
> I tend to agree, I've found conflicting info out there regarding
> PAL vs. NTSC field order. And I've never liked having to guess
> at input analog standard based on input # lines. I will go ahead
> and remove the field order override code.

Note that the code in ipu_csi_init_interface currently hard-codes field
order depending on frame size. It could be that selecting opposite field
order is as easy as switching the relevant parts of writes to registers
CCIR_CODE_2 and _3, but we'd have to pass the desired output field order
to this function. I'd welcome if somebody would verify that this works.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Philipp Zabel <pza@pengutronix.de> writes:

> Note that the code in ipu_csi_init_interface currently hard-codes field
> order depending on frame size. It could be that selecting opposite field
> order is as easy as switching the relevant parts of writes to registers
> CCIR_CODE_2 and _3, but we'd have to pass the desired output field order
> to this function. I'd welcome if somebody would verify that this works.

I can test anything I guess.
Though, in this case, I would be surprised if there is something else
needed. We already do the opposite field order by switching the
CCIR_CODE_[23] values :-)
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 05/30/2018 11:29 PM, Philipp Zabel wrote:
> On Wed, May 30, 2018 at 01:56:34PM -0700, Steve Longerbeam wrote:
>>
>> On 05/30/2018 11:46 AM, Krzysztof Hałasa wrote:
>>> Steve Longerbeam <slongerbeam@gmail.com> writes:
>>>
>>>>> but it should be possible for the user to explicitly request the field
>>>>> order on CSI output (I can make a patch I guess).
>>>> If you think that is the correct behavior, I will remove the override
>>>> code. I suppose it makes sense to allow user to select field order even
>>>> if that order does not make sense given the input standard. I'm fine
>>>> either way, Philipp what is your opinion? I'll go with the popular vote :)
>>> I think it should be up to the user.
>>> Actually, PAL and NTSC aren't valid names in the digital world. Their
>>> meaning ends in the ADV7180 (or equivalent). I don't know if PAL and/or
>>> NTSC specify the field order in the analog frame (meaningful when
>>> someone hooks a camera with progressive sensor and analog, interlaced
>>> output), but the digital YUV422 from ADV to CSI isn't NTSC/PAL anymore.
>>> It's just WxH @ framerate + possible interlacing, sequential fields,
>>> top-bottom or otherwise, etc. The analog standard names could be used
>>> here but just as defaults.
>>>
>>> If we were strict (and we don't want to force it), then we should set
>>> NTSC/PAL thing on ADV7180 input, 720x480@29.97i (or 720x576@50i, or
>>> 704x... etc) on the input parts of the CSI/IPU (where there are no video
>>> frames yet), and 720x480@29.97i B-T or T-B (or default, or separate
>>> fields - whatever suits the user) on the output from CSI.
>>>
>>> I remember the reversed field order was sometimes needed - for example,
>>> PAL DV (the casette camcorder thing) produced B-T frames (same as NTSC),
>>> and to avoid (slight) additional quality loss one had to process it
>>> (up to e.g. .MP4, DVD, and then to HDMI, SCART etc) as B-T.
>>> It wasn't a problem in otherwise-PAL-centric environment.
>> I tend to agree, I've found conflicting info out there regarding
>> PAL vs. NTSC field order. And I've never liked having to guess
>> at input analog standard based on input # lines. I will go ahead
>> and remove the field order override code.
> Note that the code in ipu_csi_init_interface currently hard-codes field
> order depending on frame size. It could be that selecting opposite field
> order is as easy as switching the relevant parts of writes to registers
> CCIR_CODE_2 and _3, but we'd have to pass the desired output field order
> to this function. I'd welcome if somebody would verify that this works.

As I said in the other thread, I think we should put this off to some
other time, and remove the code in ipu_csi_init_interface() that
inverts field order according to frame size. This way, CSI will not
be lying to userspace when we tell it the order is BT but the CSI
has actually inverted that to TB.

Also I have concerns about the CSI capturing field 1 _before_ field
0 for NTSC. Doesn't that mean the CSI will drop the B-field in the
first captured frame on stream on, and thereafter mix fields from
different adjacent frames?

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Sat, 2018-06-02 at 10:33 -0700, Steve Longerbeam wrote:
[...]
> As I said in the other thread, I think we should put this off to some
> other time, and remove the code in ipu_csi_init_interface() that
> inverts field order according to frame size. This way, CSI will not
> be lying to userspace when we tell it the order is BT but the CSI
> has actually inverted that to TB.
> 
> Also I have concerns about the CSI capturing field 1 _before_ field
> 0 for NTSC. Doesn't that mean the CSI will drop the B-field in the
> first captured frame on stream on, and thereafter mix fields from
> different adjacent frames?

Yes, that is only a problem for 29.97 Hz progressive source material.
For real 59.94 Hz interlaced source material it does not matter which
two consecutive fields are displayed together as long as we get the
top/bottom ordering right.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

> I tend to agree, I've found conflicting info out there regarding
> PAL vs. NTSC field order. And I've never liked having to guess
> at input analog standard based on input # lines. I will go ahead
> and remove the field order override code.

I've merged your current fix-csi-interlaced.2 branch (2018-06-01
00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
"media: adv7180: fix field type" commit and NTSC camera:

media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"

correctly sets:

"adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
"ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]

but all 3 cases seem to produce top-first interlaced frames.
The CCIR_CODE_* register dump shows no differences:
2a38014: 010D07DF 00040596 00FF0000

...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
registers depending on the height of the image. Hovewer, I'm using 480
lines here, so it should be B-T instead. My guess is the CSI is skipping
the first incomplete line (half-line - the first visible line has full
length) and BT becomes TB.

It seems writing to the CCIR_CODE_[12] registers does the trick, though
(the captured frames aren't correct and have the lines swapped in pairs
because t/b field pointers aren't correctly set).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

Hi Krzysztof,

On Fri, 2018-06-01 at 12:02 +0200, Krzysztof Hałasa wrote:
> Steve Longerbeam <slongerbeam@gmail.com> writes:
> 
> > I tend to agree, I've found conflicting info out there regarding
> > PAL vs. NTSC field order. And I've never liked having to guess
> > at input analog standard based on input # lines. I will go ahead
> > and remove the field order override code.
> 
> I've merged your current fix-csi-interlaced.2 branch (2018-06-01
> 00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
> "media: adv7180: fix field type" commit and NTSC camera:
> 
> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"
> 
> correctly sets:
> 
> "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
> "ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]
> 
> but all 3 cases seem to produce top-first interlaced frames.
> The CCIR_CODE_* register dump shows no differences:
> 2a38014: 010D07DF 00040596 00FF0000
> 
> ...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
> registers depending on the height of the image.

Exactly.

>  Hovewer, I'm using 480
> lines here, so it should be B-T instead.

My understanding is that the CCIR codes for height == 480 (NTSC)
currently capture the second field (top) first, assuming that for NTSC
the EAV/SAV codes are bottom-field-first.

So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
field that is captured first, where F=1 is the field that is marked as
second field on the wire, so top. Which means that the captured frame
has two fields captured across frame boundaries, which might be
problematic if the source data was originally progressive.

>  My guess is the CSI is skipping
> the first incomplete line (half-line - the first visible line has full
> length) and BT becomes TB.

That wouldn't make BT TB though, if we'd still capture the bottom field
(minus its first half line) first?

> It seems writing to the CCIR_CODE_[12] registers does the trick, though
> (the captured frames aren't correct and have the lines swapped in pairs
> because t/b field pointers aren't correctly set).

What are you writing exactly? 0x01040596 to CCIR_CODE_1 and 0x000d07df
to CCIR_CODE_2? That is what I would expect to capture SEQ_BT for NTSC
data, and the IPU could interweave this into INTERLACED_BT, correctly if
we fix ipu_cpmem_interlaced_scan to allow negative offsets.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 06/01/2018 06:13 AM, Philipp Zabel wrote:
> Hi Krzysztof,
>
> On Fri, 2018-06-01 at 12:02 +0200, Krzysztof Hałasa wrote:
>> Steve Longerbeam <slongerbeam@gmail.com> writes:
>>
>>> I tend to agree, I've found conflicting info out there regarding
>>> PAL vs. NTSC field order. And I've never liked having to guess
>>> at input analog standard based on input # lines. I will go ahead
>>> and remove the field order override code.
>> I've merged your current fix-csi-interlaced.2 branch (2018-06-01
>> 00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
>> "media: adv7180: fix field type" commit and NTSC camera:
>>
>> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
>> media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
>> media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"
>>
>> correctly sets:
>>
>> "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
>> "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
>> "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
>> "ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
>> "ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]
>>
>> but all 3 cases seem to produce top-first interlaced frames.
>> The CCIR_CODE_* register dump shows no differences:
>> 2a38014: 010D07DF 00040596 00FF0000
>>
>> ...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
>> registers depending on the height of the image.
> Exactly.
>
>>   Hovewer, I'm using 480
>> lines here, so it should be B-T instead.
> My understanding is that the CCIR codes for height == 480 (NTSC)
> currently capture the second field (top) first, assuming that for NTSC
> the EAV/SAV codes are bottom-field-first.
>
> So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
> in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
> field that is captured first, where F=1 is the field that is marked as
> second field on the wire, so top. Which means that the captured frame
> has two fields captured across frame boundaries, which might be
> problematic if the source data was originally progressive.

I agree, for NTSC the CSI will drop the first B field and start capturing
at the T field, and then capture fields across frame boundaries. At
least, that is if we understand how these CCIR registers work: the
CSI will look for H-S-V codes for the start and end of active and blanking
lines, that match the codes written to CCIR_CODE_1/2 for fields 0/1.

I think this must be legacy code from a Freescale BSP requirement
that the CSI must always capture in T-B order. We should remove this
code, so that the CSI always captures field 0 followed by field 1, 
irrespective
of field affinity, as in:

diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
index 5450a2d..b8b9b6d 100644
--- a/drivers/gpu/ipu-v3/ipu-csi.c
+++ b/drivers/gpu/ipu-v3/ipu-csi.c
@@ -398,41 +398,20 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
                 break;
         case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
                 if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
-                       /*
-                        * PAL case
-                        *
-                        * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
-                        * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
-                        * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
-                        * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
-                        */
                         height = 625; /* framelines for PAL */
-
-                       ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
-                                         CSI_CCIR_CODE_1);
-                       ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
-                       ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
                 } else if (mbus_fmt->width == 720 && mbus_fmt->height 
== 480) {
-                       /*
-                        * NTSC case
-                        *
-                        * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
-                        * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
-                        * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
-                        * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
-                        */
                         height = 525; /* framelines for NTSC */
-
-                       ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
-                                         CSI_CCIR_CODE_1);
-                       ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
-                       ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
                 } else {
                         dev_err(csi->ipu->dev,
                                 "Unsupported CCIR656 interlaced video 
mode\n");
                         spin_unlock_irqrestore(&csi->lock, flags);
                         return -EINVAL;
                 }
+
+               ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
+                             CSI_CCIR_CODE_1);
+               ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
+               ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
                 break;
         case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_DDR:
         case IPU_CSI_CLK_MODE_CCIR1120_PROGRESSIVE_SDR:

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

> I think this must be legacy code from a Freescale BSP requirement
> that the CSI must always capture in T-B order. We should remove this
> code, so that the CSI always captures field 0 followed by field 1,
> irrespective
> of field affinity,

Well it now seems we could do just that.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Sat, 2018-06-02 at 10:45 -0700, Steve Longerbeam wrote:
> 
> On 06/01/2018 06:13 AM, Philipp Zabel wrote:
> > Hi Krzysztof,
> > 
> > On Fri, 2018-06-01 at 12:02 +0200, Krzysztof Hałasa wrote:
> > > Steve Longerbeam <slongerbeam@gmail.com> writes:
> > > 
> > > > I tend to agree, I've found conflicting info out there regarding
> > > > PAL vs. NTSC field order. And I've never liked having to guess
> > > > at input analog standard based on input # lines. I will go ahead
> > > > and remove the field order override code.
> > > 
> > > I've merged your current fix-csi-interlaced.2 branch (2018-06-01
> > > 00:06:45 UTC 22ad9f30454b6e46979edf6f8122243591910a3e) along with
> > > "media: adv7180: fix field type" commit and NTSC camera:
> > > 
> > > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:alternate]"
> > > media-ctl -V "'ipu2_csi1_mux':2 [fmt:UYVY2X8/720x480]"
> > > media-ctl -V "'ipu2_csi1':2 [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]"
> > > 
> > > correctly sets:
> > > 
> > > "adv7180 2-0020":0 [fmt:UYVY2X8/720x480 field:alternate]
> > > "ipu2_csi1_mux":1  [fmt:UYVY2X8/720x480 field:alternate]
> > > "ipu2_csi1_mux":2  [fmt:UYVY2X8/720x480 field:alternate]
> > > "ipu2_csi1":0      [fmt:UYVY2X8/720x480 field:alternate]
> > > "ipu2_csi1":2      [fmt:AYUV32/720x480 field:interlaced/-bt/-tb]
> > > 
> > > but all 3 cases seem to produce top-first interlaced frames.
> > > The CCIR_CODE_* register dump shows no differences:
> > > 2a38014: 010D07DF 00040596 00FF0000
> > > 
> > > ...it's because the code in drivers/gpu/ipu-v3/ipu-csi.c still sets the
> > > registers depending on the height of the image.
> > 
> > Exactly.
> > 
> > >   Hovewer, I'm using 480
> > > lines here, so it should be B-T instead.
> > 
> > My understanding is that the CCIR codes for height == 480 (NTSC)
> > currently capture the second field (top) first, assuming that for NTSC
> > the EAV/SAV codes are bottom-field-first.
> > 
> > So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
> > in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
> > field that is captured first, where F=1 is the field that is marked as
> > second field on the wire, so top. Which means that the captured frame
> > has two fields captured across frame boundaries, which might be
> > problematic if the source data was originally progressive.
> 
> I agree, for NTSC the CSI will drop the first B field and start capturing
> at the T field, and then capture fields across frame boundaries. At
> least, that is if we understand how these CCIR registers work: the
> CSI will look for H-S-V codes for the start and end of active and blanking
> lines, that match the codes written to CCIR_CODE_1/2 for fields 0/1.
> 
> I think this must be legacy code from a Freescale BSP requirement
> that the CSI must always capture in T-B order. We should remove this
> code, so that the CSI always captures field 0 followed by field 1, 
> irrespective
> of field affinity, as in:
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-csi.c b/drivers/gpu/ipu-v3/ipu-csi.c
> index 5450a2d..b8b9b6d 100644
> --- a/drivers/gpu/ipu-v3/ipu-csi.c
> +++ b/drivers/gpu/ipu-v3/ipu-csi.c
> @@ -398,41 +398,20 @@ int ipu_csi_init_interface(struct ipu_csi *csi,
>                  break;
>          case IPU_CSI_CLK_MODE_CCIR656_INTERLACED:
>                  if (mbus_fmt->width == 720 && mbus_fmt->height == 576) {
> -                       /*
> -                        * PAL case
> -                        *
> -                        * Field0BlankEnd = 0x6, Field0BlankStart = 0x2,
> -                        * Field0ActiveEnd = 0x4, Field0ActiveStart = 0
> -                        * Field1BlankEnd = 0x7, Field1BlankStart = 0x3,
> -                        * Field1ActiveEnd = 0x5, Field1ActiveStart = 0x1
> -                        */
>                          height = 625; /* framelines for PAL */
> -
> -                       ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> -                                         CSI_CCIR_CODE_1);
> -                       ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> -                       ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>                  } else if (mbus_fmt->width == 720 && mbus_fmt->height 
> == 480) {
> -                       /*
> -                        * NTSC case
> -                        *
> -                        * Field0BlankEnd = 0x7, Field0BlankStart = 0x3,
> -                        * Field0ActiveEnd = 0x5, Field0ActiveStart = 0x1
> -                        * Field1BlankEnd = 0x6, Field1BlankStart = 0x2,
> -                        * Field1ActiveEnd = 0x4, Field1ActiveStart = 0
> -                        */
>                          height = 525; /* framelines for NTSC */
> -
> -                       ipu_csi_write(csi, 0xD07DF | CSI_CCIR_ERR_DET_EN,
> -                                         CSI_CCIR_CODE_1);
> -                       ipu_csi_write(csi, 0x40596, CSI_CCIR_CODE_2);
> -                       ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);
>                  } else {
>                          dev_err(csi->ipu->dev,
>                                  "Unsupported CCIR656 interlaced video 
> mode\n");
>                          spin_unlock_irqrestore(&csi->lock, flags);
>                          return -EINVAL;
>                  }
> +
> +               ipu_csi_write(csi, 0x40596 | CSI_CCIR_ERR_DET_EN,
> +                             CSI_CCIR_CODE_1);
> +               ipu_csi_write(csi, 0xD07DF, CSI_CCIR_CODE_2);
> +               ipu_csi_write(csi, 0xFF0000, CSI_CCIR_CODE_3);

This will require a negative interlace offset in the IDMAC to produce
seq-bt -> interlaced-bt for NTSC.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

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 Halasa
Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Tim Harvey]

On Mon, Jun 4, 2018 at 1:58 AM Krzysztof Hałasa <khalasa@piap.pl> 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?

Steve, I haven't tried your 'media: imx: Switch to subdev notifiers'
v7 series yet (https://patchwork.kernel.org/cover/10620967/) but can
certainly do so if you need testing. I'm hoping those changes are all
internal and won't affect the userspace pipeline configuration between
kernel versions?

I'm also interested in looking at Philipps' 'i.MX media mem2mem
scaler' series (https://patchwork.kernel.org/cover/10603881/) and am
wondering if anyone has some example pipelines showing that in use.
I'm hoping that is what is needed to be able to use hardware
scaling/CSC and coda based encoding on streams from v4l2 PCI capture
devices.

Lastly, is there any hope to use IMX6 hardware compositing to say
stitch together multiple streams from a v4l2 PCI capture device into a
single stream for coda based hw encoding?

Regards,

Tim

Re: i.MX6 IPU CSI analog video input on Ventana

[Tim Harvey]

On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam <slongerbeam@gmail.com> 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@piap.pl> 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?

# 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?

I haven't enabled FIM  yet and don't recall how to do so from
userspace now that its using V4L2 CID's. 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?

Tim

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 10/17/18 4:05 PM, Tim Harvey wrote:
> On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam <slongerbeam@gmail.com> 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@piap.pl> 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.


>
> # 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")


>
> 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

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.

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Tim Harvey]

On Wed, Oct 17, 2018 at 4:37 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>
> On 10/17/18 4:05 PM, Tim Harvey wrote:
> > On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam <slongerbeam@gmail.com> 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@piap.pl> 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

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

On 10/18/18 10:56 AM, Tim Harvey wrote:
> On Wed, Oct 17, 2018 at 4:37 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>
>> On 10/17/18 4:05 PM, Tim Harvey wrote:
>>> On Wed, Oct 17, 2018 at 2:33 PM Steve Longerbeam <slongerbeam@gmail.com> 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@piap.pl> 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:


Sorry right, forgot to mention adv7180.c needs to implement
.g_skip_frames. The below patch is the right idea.


> 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.


Yes, FIM is another solution for dealing with the unstable initial
bt.656 sync codes. But with the current drawback that the EOF
method is subject to irq latency error, so it's possible to trigger
false FIM events by, say attaching a USB device creating a flurry
of IO. Also FIM requires participation from userland (send a stream
restart in response to the FIM event), and a gstreamer pipeline,
e.g. v4l2src won't do this.


>
> 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.".


The push-back was not about using g_skip_frames. It was in
reaching through the media graph to search for the backend
sensor.


>
> 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?


Sure, I will look into trying again. It would have to be a chained
approach, similar to how .s_steam is chained.


>
> Or perhaps FIM resolves both of these issue?


It does but with the above drawbacks mentioned.


>
>>> 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?


I'm not using a patched version, referencing controls by
name has been part of v4l2-ctl for a long time AFAIK.
But remember FIM controls are only available from the
ipu_csi capture device nodes ATM, not the scaling/CSC/rotation
ipu_ic_prp pipelines.


Steve

>
>> 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

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Wed, 2018-10-17 at 14:33 -0700, Steve Longerbeam wrote:
[...]
> > I'm also interested in looking at Philipps' 'i.MX media mem2mem
> > scaler' series (https://patchwork.kernel.org/cover/10603881/) and am
> > wondering if anyone has some example pipelines showing that in use.
> > I'm hoping that is what is needed to be able to use hardware
> > scaling/CSC and coda based encoding on streams from v4l2 PCI capture
> > devices.
> 
> Yes exactly, I'll let Philipp answer. I'm also interested in the gstreamer
> element needed to make use of h/w scaling/CSC from the mem2mem
> device.

GStreamer should create a GstV4l2Transform element "v4l2videoXconvert"
for the /dev/videoX mem2mem scaler device.

> For coda encode, my understanding is that the v4l2h264enc element will
> make use of coda h/w encode, something like this example which encodes to
> a h.264 file (I haven't verified this works, still need to build a later 
> version of gst-plugins-good that has the vl2h264enc support):
> 
> gst-launch-1.0 v4l2src io-mode=dmabuf device=/dev/video$dev !\ "video/x-raw,format=$fmt,width=$w,height=$h"  ! \
> v4l2h264enc output-io-mode=dmabuf-import  ! queue ! matroskamux ! \
> filesink location=$filename

With GStreamer 1.14 the capture side io-mode parameter is not necessary
anymore to export dmabufs.
The output-io-mode parameter is currently still needed though, as the
V4L2 elements don't support negotiating dmabuf using caps via
video/x-raw(memory:DMABuf) yet.

Also there's a h264parse missing to convert the video/x-h264,stream-
format=byte-stream from v4l2h264enc to video/x-h264,stream-format=avc as
required by matroskamux:

gst-launch-1.0 \
	v4l2src ! \
	v4l2video10convert output-io-mode=dmabuf-import ! \
	v4l2h264enc output-io-mode=dmabuf-import ! \
	h264parse ! \
	matroskamux ! \
	filesink

> > Lastly, is there any hope to use IMX6 hardware compositing to say
> > stitch together multiple streams from a v4l2 PCI capture device into a
> > single stream for coda based hw encoding?
> 
> The IPUv3 Image Converter has a combining unit that can combine pixels from
> two images, but there is no support for that in mainline AFAIK.

I don't think there is any V4L2 API for compositing yet.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Hi Philipp,

Philipp Zabel <p.zabel@pengutronix.de> writes:

> My understanding is that the CCIR codes for height == 480 (NTSC)
> currently capture the second field (top) first, assuming that for NTSC
> the EAV/SAV codes are bottom-field-first.

2a38014: 010D07DF 00040596

        SA  EA         SB  EB  SB  EB
D07DF: 001 101 (0000) 011 111 011 111 (field 0)
40596: 000 100 (0000) 010 110 010 110 (field 1)

The codes apparently are 1=EAV (0=SAV), field#, 1=blanking.
Now BT.656 doesn't say a word about top and bottom fields. There are
just fields 1 and 2. So yes, the CCIR_CODE* registers currently seem to
swap the fields. It also depends on the ADV7180 sending correct codes
based on the even/odd analog fields. Interesting.

> So the CSI captures SEQ_TB for both PAL and NTSC: The three-bit values
> in CCIR_CODE_2/3 are in H,V,F order, and the NTSC case has F=1 for the
> field that is captured first, where F=1 is the field that is marked as
> second field on the wire, so top. Which means that the captured frame
> has two fields captured across frame boundaries, which might be
> problematic if the source data was originally progressive.

Exactly.
Especially if the complete frame is then passed straight to the display,
with the user treating it as progressive (which it isn't anymore).

>>  My guess is the CSI is skipping
>> the first incomplete line (half-line - the first visible line has full
>> length) and BT becomes TB.
>
> That wouldn't make BT TB though, if we'd still capture the bottom field
> (minus its first half line) first?

Well, the entire frame would shift up a line, the bottom "field" would
become top and vice versa. This would effectively make BT->TB and TB->BT.

>> It seems writing to the CCIR_CODE_[12] registers does the trick, though
>> (the captured frames aren't correct and have the lines swapped in pairs
>> because t/b field pointers aren't correctly set).
>
> What are you writing exactly? 0x01040596 to CCIR_CODE_1 and 0x000d07df
> to CCIR_CODE_2?

Yes.

> That is what I would expect to capture SEQ_BT for NTSC
> data, and the IPU could interweave this into INTERLACED_BT, correctly if
> we fix ipu_cpmem_interlaced_scan to allow negative offsets.

Exactly.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Steve Longerbeam]

Hi Philipp,


On 05/24/2018 11:32 PM, Philipp Zabel wrote:
> On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
> [...]
>>> The following is required as well. Now the question is why we can't skip
>>> writing those odd UV rows. Anyway, with these 2 changes, I get a stable
>>> NTSC (and probably PAL) interlaced video stream.
>>>
>>> The manual says: Reduce Double Read or Writes (RDRW):
>>> This bit is relevant for YUV4:2:0 formats. For write channels:
>>> U and V components are not written to odd rows.
>>>
>>> How could it be so? With YUV420, are they normally written?
>> Well, given that this bit exists, and assuming I understand it correctly
>> (1),
>> I guess the U and V components for odd rows normally are placed on the
>> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
>> the U and V components are the same for odd and even rows.
>>
>> In other words for writing 4:2:0 to memory, this bit should _always_ be set.
>>
>> (1) OTOH I don't really understand what this bit is trying to say.
>> Whether this bit is set or not, the data in memory is correct
>> for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
>> (decimated by 2 in width and height), followed by Y plane of 1/4
>> size.
>>
>> So I assume it is saying that the IPU normally places U/V components
>> on the AXI bus for odd rows, that are identical to the even row values.
> Whether they are identical depends on the input format.

Right, this is the part I was missing, thanks for clarifying. The
even and odd chroma rows coming into the IDMAC from the
CSI (or IC) may not be identical if the CSI has captured 4:4:4
(or 4:2:2 yeah? 4:2:2 is only decimated in width not height).

But still, when the IDMAC has finished pixel packing/unpacking and
is writing 4:2:0 to memory, it should always skip overwriting the even
rows with the odd rows, whether or not it has received identical chroma
even/odd lines from the CSI.

Unless interweave is enabled :) See below.
>
> The IDMAC always gets fed AYUV32 from the CSI or IC.
> If the CSI captures YUV 4:2:x, odd and even lines will have the same
> chroma values. But if the CSI captures YUV 4:4:4 (or RGB, fed through
> the IC), we can have AYUV32 input with different chroma values on even
> and odd lines.
> In that case the IPU just writes the even chroma line and then
> overwrites it with the odd line, unless the double write reduction bit
> is set.
>
>> IOW somehow those identical odd rows are dropped before writing to
>> the U/V planes in memory.
> potentially identical.

Right.

>
>> Philipp please chime in if you have something to add here.
> I suppose the bit could be used to choose to write the chroma values of
> odd instead of even lines for 4:4:4 inputs, at the cost of increased
> memory bandwidth usage.
>
>> Steve
>>
>>> OTOH it seems that not only UV is broken with this bit set.
>>> Y is broken as well.
> Maybe scanline interlave and double write reduction can't be used at the
> same time?

Yes, I just verified that. I went back to the SabreLite with the
progressive output OV5640, and double-write-reduction for
4:2:0 capture works fine, the images are correct.

Steve

Re: i.MX6 IPU CSI analog video input on Ventana

[Philipp Zabel]

On Fri, 2018-05-25 at 16:21 -0700, Steve Longerbeam wrote:
> Hi Philipp,
> 
> On 05/24/2018 11:32 PM, Philipp Zabel wrote:
> > On Thu, 2018-05-24 at 11:12 -0700, Steve Longerbeam wrote:
> > [...]
> > > > The following is required as well. Now the question is why we can't skip
> > > > writing those odd UV rows. Anyway, with these 2 changes, I get a stable
> > > > NTSC (and probably PAL) interlaced video stream.
> > > > 
> > > > The manual says: Reduce Double Read or Writes (RDRW):
> > > > This bit is relevant for YUV4:2:0 formats. For write channels:
> > > > U and V components are not written to odd rows.
> > > > 
> > > > How could it be so? With YUV420, are they normally written?
> > > 
> > > Well, given that this bit exists, and assuming I understand it correctly
> > > (1),
> > > I guess the U and V components for odd rows normally are placed on the
> > > AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> > > the U and V components are the same for odd and even rows.
> > > 
> > > In other words for writing 4:2:0 to memory, this bit should _always_ be set.
> > > 
> > > (1) OTOH I don't really understand what this bit is trying to say.
> > > Whether this bit is set or not, the data in memory is correct
> > > for planar 4:2:0: y plane buffer followed by U plane of 1/4 size
> > > (decimated by 2 in width and height), followed by Y plane of 1/4
> > > size.
> > > 
> > > So I assume it is saying that the IPU normally places U/V components
> > > on the AXI bus for odd rows, that are identical to the even row values.
> > 
> > Whether they are identical depends on the input format.
> 
> Right, this is the part I was missing, thanks for clarifying. The
> even and odd chroma rows coming into the IDMAC from the
> CSI (or IC) may not be identical if the CSI has captured 4:4:4
> (or 4:2:2 yeah? 4:2:2 is only decimated in width not height).

Oh right, the MEDIA_BUS_FMT_YUYV variants are pretty common, they have
chroma for all the lines. Actually, that is my default test case (1080p
YUYV from TC358743), usually written to NV12 so it can be encoded.

> But still, when the IDMAC has finished pixel packing/unpacking and
> is writing 4:2:0 to memory, it should always skip overwriting the even
> rows with the odd rows, whether or not it has received identical chroma
> even/odd lines from the CSI.
> 
> Unless interweave is enabled :) See below.

Agreed.

regards
Philipp

Re: i.MX6 IPU CSI analog video input on Ventana

[Krzysztof Hałasa]

Steve Longerbeam <slongerbeam@gmail.com> writes:

>> The manual says: Reduce Double Read or Writes (RDRW):
>> This bit is relevant for YUV4:2:0 formats. For write channels:
>> U and V components are not written to odd rows.
>>
>> How could it be so? With YUV420, are they normally written?
>
> Well, given that this bit exists, and assuming I understand it
> correctly (1),
> I guess the U and V components for odd rows normally are placed on the
> AXI bus. Which is a total waste of bus bandwidth because in 4:2:0,
> the U and V components are the same for odd and even rows.

Right. Now, the AXI bus is just a "memory bus", it's a newer version of
the AHB. One can't simply "place data" on AXI, it must be a write to
a specific address, and the data will end up in RAM (assuming the
configuration is sane). How can we have two possible data formats (with
and without the RDRW bit) with fixed image format (420-type) is beyond
me.

> Commits
>
> 14330d7f08 ("media: imx: csi: enable double write reduction")
> b54a5c2dc8 ("media: imx: prpencvf: enable double write reduction")
>
> should be reverted for now, until the behavior of this bit is better
> understood.

I agree.

I have dumped a raw frame (720 x 480 NV12 frame size 518400 from
interlaced NTSC camera), with the RDRW bit set.

The Y plane contains, well, valid Y data (720 x 480 bytes).

The color plane (360 pixels x 240 line pairs * 2 colors) has every other
line pair zeroed. I.e., there is a 720-byte line pair filled with valid UV
data, then there are 720 zeros (360 zeroed UV pairs). Then there is valid
UV data and so on.

Not sure what could it be for. Some weird sort of YUV 4:1:0? I guess we
don't want it ATM.

WRT ADV7180 field format:

> This might be a good time to bring up the fact that the ADV7180 driver
> is wrong
> to set output to "interlaced". The ADV7180 does not transmit top lines
> interlaced
> with bottom lines. It transmits all top lines followed by all bottom
> lines (or
> vice-versa), i.e. it should be either V4L2_FIELD_SEQ_TB or
> V4L2_FIELD_SEQ_BT.
> It can also be set to V4L2_FIELD_ALTERNATE, and then it is left up to
> downstream
> elements to determine field order (TB or BT).

Right. ADV7180, AFAIK, doesn't have the hardware (frame buffer) to get
two interlaced fields and merge them to form a complete frame.
It simply transforms the incoming analog signal into binary data stream.
This issue should be fixed.

Thanks for your work,
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: i.MX6 IPU CSI analog video input on Ventana

[Franz Melchior]

Hey,

* Krzysztof Hałasa, 2018-05-21 10:09:
> I wonder... perhaps to get an interlaced frame I need to route the data
> through VDIC (ipu2_vdic, the deinterlacer)?

I have the same problem here, though not investigated nearly as
thoroughly as you did, down to the IPU register values. But I also
get the images stable (PAL and NTSC) with every other line green
when de-interlacing via PRPVF/IDMAC. Unfortunately, de-interlacing
via VDIC doesn't work much better:

While the image seems properly de-interlaced and colored then, it
keeps jumping up an down a few times per second, because the first
line isn't always included in the image. (The pipeline is the same
as Tim showed under "using VDIC to deinterlace").

That's with the adv7180 and an iMX6Q with kernel 4.14.35.

m.

-- 
Melchior Franz | Entwicklung Software

GINZINGER ELECTRONIC SYSTEMS GMBH

Tel.: +43 7723 5422 156
Mail: melchior.franz@ginzinger.com
Web: www.ginzinger.com

Re: i.MX6 IPU CSI analog video input on Ventana

[Franz Melchior]

Hey,

* Krzysztof Hałasa, 2018-05-10 10:19:
> I'm using analog PAL video in on GW53xx/54xx boards (through ADV7180
> chip and 8-bit parallel CSI input, with (presumably) BT.656).
[...]
> First, I can't find a way to change to PAL standard. *s_std() doesn't
> propagate from "ipu2_csi1 capture" through "ipu2_csi1_mux" to adv7180.
>
> For now I have just changed the default:
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -1320,7 +1321,7 @@ static int adv7180_probe(struct i2c_client *client,
>
>       state->irq = client->irq;
>       mutex_init(&state->mutex);
> -    state->curr_norm = V4L2_STD_NTSC;
> +    state->curr_norm = V4L2_STD_PAL;
>       if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
>           state->powered = true;
>       else

JFTR: I had a similar problem on a board, where there can either be an
NTSC *or* a PAL camera, so I had to make it dynamic.


diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 3df28f2f9b38..e5ebebf7a1f4 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1320,7 +1320,27 @@ static int adv7180_probe(struct i2c_client *client,

         state->irq = client->irq;
         mutex_init(&state->mutex);
+
+       /* check if video standard (PAL, NTSC) has already been determined */
+       ret = adv7180_read(state, ADV7180_REG_STATUS1);
+       if (ret < 0) {
+               ret = -EIO;
+               goto err_unregister_vpp_client;
+       }
+
         state->curr_norm = V4L2_STD_NTSC;
+       if (ret & ADV7180_STATUS1_IN_LOCK) {
+               ret = (ret >> 4) & 0x07; /* autodetection result (AD_RESULT) */
+               if (ret >= 2 && ret <= 4) {
+                       v4l_info(client, "locked on PAL signal\n");
+                       state->curr_norm = V4L2_STD_PAL;
+               } else {
+                       v4l_info(client, "locked on NTSC signal\n");
+               }
+       } else {
+               v4l_info(client, "no signal, using NTSC\n");
+       }
+
         if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
                 state->powered = true;
         else
m.


Melchior Franz | Entwicklung Software

GINZINGER ELECTRONIC SYSTEMS GMBH

Tel.: +43 7723 5422 156
Mail: melchior.franz@ginzinger.com
Web: www.ginzinger.com







________________________________________

Ginzinger electronic systems GmbH
Gewerbegebiet Pirath 16
4952 Weng im Innkreis
www.ginzinger.com

Firmenbuchnummer: FN 364958d
Firmenbuchgericht: Ried im Innkreis
UID-Nr.: ATU66521089