diff mbox series

[v8,11/11] media: imx.rst: Update doc to reflect fixes to interlaced capture

Message ID 20190109183014.20466-12-slongerbeam@gmail.com (mailing list archive)
State New, archived
Headers show
Series imx-media: Fixes for interlaced capture | expand

Commit Message

Steve Longerbeam Jan. 9, 2019, 6:30 p.m. UTC
Also add an example pipeline for unconverted capture with interweave
on SabreAuto.

Cleanup some language in various places in the process.

Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
---
Changes since v4:
- Make clear that it is IDMAC channel that does pixel reordering and
  interweave, not the CSI. Caught by Philipp Zabel.
Changes since v3:
- none.
Changes since v2:
- expand on idmac interweave behavior in CSI subdev.
- switch second SabreAuto pipeline example to PAL to give
  both NTSC and PAL examples.
- Cleanup some language in various places.
---
 Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
 1 file changed, 66 insertions(+), 37 deletions(-)

Comments

Tim Harvey Jan. 15, 2019, 9:58 p.m. UTC | #1
On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Also add an example pipeline for unconverted capture with interweave
> on SabreAuto.
>
> Cleanup some language in various places in the process.
>
> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
> Changes since v4:
> - Make clear that it is IDMAC channel that does pixel reordering and
>   interweave, not the CSI. Caught by Philipp Zabel.
> Changes since v3:
> - none.
> Changes since v2:
> - expand on idmac interweave behavior in CSI subdev.
> - switch second SabreAuto pipeline example to PAL to give
>   both NTSC and PAL examples.
> - Cleanup some language in various places.
> ---
>  Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
>  1 file changed, 66 insertions(+), 37 deletions(-)
>
<snip>
>  Capture Pipelines
>  -----------------
> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>  parallel bus input on the internal video mux to IPU1 CSI0.
>
>  The following example configures a pipeline to capture from the ADV7180
> -video decoder, assuming NTSC 720x480 input signals, with Motion
> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
> -entity at its output pad:
> +video decoder, assuming NTSC 720x480 input signals, using simple
> +interweave (unconverted and without motion compensation). The adv7180
> +must output sequential or alternating fields (field type 'seq-bt' for
> +NTSC, or 'alternate'):
> +
> +.. code-block:: none
> +
> +   # Setup links
> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> +   # Configure pads
> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
> +
> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
> +used to select any supported YUV pixelformat on /dev/video4.
> +

Hi Steve,

I'm testing 4.20 with this patchset on top.

I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
the SabeAuto example above I can't get the simple interveave example
to work:

media-ctl -r # reset all links
# Setup links (ADV7180 IPU1_CSI0)
media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
# Configure pads
media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
# Configure 'ipu1_csi0 capture' interface (/dev/video4)
v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
# streaming can now begin on the raw capture device node at /dev/video4
v4l2-ctl -d4 --stream-mmap --stream-to=/x.raw --stream-count=1 # capture 1 frame
[ 5547.354460] ipu1_csi0: pipeline start failed with -32
VIDIOC_STREAMON: failed: Broken pipe

Any ideas what is causing this pipeline failure.

> +This example configures a pipeline to capture from the ADV7180
> +video decoder, assuming PAL 720x576 input signals, with Motion
> +Compensated de-interlacing. The adv7180 must output sequential or
> +alternating fields (field type 'seq-tb' for PAL, or 'alternate').
> +$outputfmt can be any format supported by the ipu1_ic_prpvf entity
> +at its output pad:
>
>  .. code-block:: none
>
> @@ -531,11 +560,11 @@ entity at its output pad:
>     media-ctl -l "'ipu1_ic_prp':2 -> 'ipu1_ic_prpvf':0[1]"
>     media-ctl -l "'ipu1_ic_prpvf':1 -> 'ipu1_ic_prpvf capture':0[1]"
>     # Configure pads
> -   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480]"
> -   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]"
> -   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x480 field:interlaced]"
> -   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x480 field:none]"
> -   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x480 field:none]"
> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
> +   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x576]"
> +   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x576 field:none]"
> +   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x576 field:none]"
>     media-ctl -V "'ipu1_ic_prpvf':1 [fmt:$outputfmt field:none]"
>
>  Streaming can then begin on the capture device node at

The above motion-compensation example pipeline does now work with this
patch series - thanks for addressing this!

Regards,

Tim
Steve Longerbeam Jan. 15, 2019, 11:54 p.m. UTC | #2
Hi Tim,

On 1/15/19 1:58 PM, Tim Harvey wrote:
> On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Also add an example pipeline for unconverted capture with interweave
>> on SabreAuto.
>>
>> Cleanup some language in various places in the process.
>>
>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>> ---
>> Changes since v4:
>> - Make clear that it is IDMAC channel that does pixel reordering and
>>    interweave, not the CSI. Caught by Philipp Zabel.
>> Changes since v3:
>> - none.
>> Changes since v2:
>> - expand on idmac interweave behavior in CSI subdev.
>> - switch second SabreAuto pipeline example to PAL to give
>>    both NTSC and PAL examples.
>> - Cleanup some language in various places.
>> ---
>>   Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
>>   1 file changed, 66 insertions(+), 37 deletions(-)
>>
> <snip>
>>   Capture Pipelines
>>   -----------------
>> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>>   parallel bus input on the internal video mux to IPU1 CSI0.
>>
>>   The following example configures a pipeline to capture from the ADV7180
>> -video decoder, assuming NTSC 720x480 input signals, with Motion
>> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
>> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
>> -entity at its output pad:
>> +video decoder, assuming NTSC 720x480 input signals, using simple
>> +interweave (unconverted and without motion compensation). The adv7180
>> +must output sequential or alternating fields (field type 'seq-bt' for
>> +NTSC, or 'alternate'):
>> +
>> +.. code-block:: none
>> +
>> +   # Setup links
>> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
>> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
>> +   # Configure pads
>> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
>> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
>> +
>> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
>> +used to select any supported YUV pixelformat on /dev/video4.
>> +
> Hi Steve,
>
> I'm testing 4.20 with this patchset on top.
>
> I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
> the SabeAuto example above I can't get the simple interveave example
> to work:
>
> media-ctl -r # reset all links
> # Setup links (ADV7180 IPU1_CSI0)
> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
> media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
> # Configure pads
> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"

This is the reason. The adv7180 is only allowing to configure alternate 
field mode, and thus it reports the field height on the mbus, not the 
full frame height. Imx deals with alternate field mode by capturing a 
full frame, so the CSI entity sets the output pad height to double the 
height.

So the CSI input pad needs to be configured with the field height:

media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"

It should work for you after doing that. And better yet, don't bother 
configuring the input pad, because media-ctl will propagate formats from 
source to sink pads for you, so it's better to rely on the propagation, 
and set the CSI output pad format instead (full frame height at output pad):

media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"


Final note: the imx.rst doc is technically correct even though it is 
showing full frame heights being configured at the pads, because it is 
expecting the adv7180 has accepted 'seq-bt'. But even the example given 
in that doc works for alternate field mode, because the pad heights are 
forced to the correct field height for alternate mode.

Steve



> # Configure 'ipu1_csi0 capture' interface (/dev/video4)
> v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
> # streaming can now begin on the raw capture device node at /dev/video4
> v4l2-ctl -d4 --stream-mmap --stream-to=/x.raw --stream-count=1 # capture 1 frame
> [ 5547.354460] ipu1_csi0: pipeline start failed with -32
> VIDIOC_STREAMON: failed: Broken pipe
>
> Any ideas what is causing this pipeline failure.
>
>> +This example configures a pipeline to capture from the ADV7180
>> +video decoder, assuming PAL 720x576 input signals, with Motion
>> +Compensated de-interlacing. The adv7180 must output sequential or
>> +alternating fields (field type 'seq-tb' for PAL, or 'alternate').
>> +$outputfmt can be any format supported by the ipu1_ic_prpvf entity
>> +at its output pad:
>>
>>   .. code-block:: none
>>
>> @@ -531,11 +560,11 @@ entity at its output pad:
>>      media-ctl -l "'ipu1_ic_prp':2 -> 'ipu1_ic_prpvf':0[1]"
>>      media-ctl -l "'ipu1_ic_prpvf':1 -> 'ipu1_ic_prpvf capture':0[1]"
>>      # Configure pads
>> -   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480]"
>> -   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]"
>> -   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x480 field:interlaced]"
>> -   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x480 field:none]"
>> -   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x480 field:none]"
>> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
>> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
>> +   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x576]"
>> +   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x576 field:none]"
>> +   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x576 field:none]"
>>      media-ctl -V "'ipu1_ic_prpvf':1 [fmt:$outputfmt field:none]"
>>
>>   Streaming can then begin on the capture device node at
> The above motion-compensation example pipeline does now work with this
> patch series - thanks for addressing this!
>
> Regards,
>
> Tim
Steve Longerbeam Jan. 15, 2019, 11:59 p.m. UTC | #3
On 1/15/19 3:54 PM, Steve Longerbeam wrote:
>
>>
>>
>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
>
> This is the reason. The adv7180 is only allowing to configure 
> alternate field mode, and thus it reports the field height on the 
> mbus, not the full frame height. Imx deals with alternate field mode 
> by capturing a full frame, so the CSI entity sets the output pad 
> height to double the height.

gah,

"... so the CSI entity sets the output pad height to double the _input 
pad_ height."

Steve
Tim Harvey Jan. 21, 2019, 8:24 p.m. UTC | #4
On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> Hi Tim,
>
> On 1/15/19 1:58 PM, Tim Harvey wrote:
> > On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >> Also add an example pipeline for unconverted capture with interweave
> >> on SabreAuto.
> >>
> >> Cleanup some language in various places in the process.
> >>
> >> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> >> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> >> ---
> >> Changes since v4:
> >> - Make clear that it is IDMAC channel that does pixel reordering and
> >>    interweave, not the CSI. Caught by Philipp Zabel.
> >> Changes since v3:
> >> - none.
> >> Changes since v2:
> >> - expand on idmac interweave behavior in CSI subdev.
> >> - switch second SabreAuto pipeline example to PAL to give
> >>    both NTSC and PAL examples.
> >> - Cleanup some language in various places.
> >> ---
> >>   Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
> >>   1 file changed, 66 insertions(+), 37 deletions(-)
> >>
> > <snip>
> >>   Capture Pipelines
> >>   -----------------
> >> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
> >>   parallel bus input on the internal video mux to IPU1 CSI0.
> >>
> >>   The following example configures a pipeline to capture from the ADV7180
> >> -video decoder, assuming NTSC 720x480 input signals, with Motion
> >> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
> >> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
> >> -entity at its output pad:
> >> +video decoder, assuming NTSC 720x480 input signals, using simple
> >> +interweave (unconverted and without motion compensation). The adv7180
> >> +must output sequential or alternating fields (field type 'seq-bt' for
> >> +NTSC, or 'alternate'):
> >> +
> >> +.. code-block:: none
> >> +
> >> +   # Setup links
> >> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
> >> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> >> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> >> +   # Configure pads
> >> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> >> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> >> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> >> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
> >> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
> >> +
> >> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
> >> +used to select any supported YUV pixelformat on /dev/video4.
> >> +
> > Hi Steve,
> >
> > I'm testing 4.20 with this patchset on top.
> >
> > I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
> > the SabeAuto example above I can't get the simple interveave example
> > to work:
> >
> > media-ctl -r # reset all links
> > # Setup links (ADV7180 IPU1_CSI0)
> > media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
> > media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> > media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
> > # Configure pads
> > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> > media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> > media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
>
> This is the reason. The adv7180 is only allowing to configure alternate
> field mode, and thus it reports the field height on the mbus, not the
> full frame height. Imx deals with alternate field mode by capturing a
> full frame, so the CSI entity sets the output pad height to double the
> height.
>
> So the CSI input pad needs to be configured with the field height:
>
> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"
>
> It should work for you after doing that. And better yet, don't bother
> configuring the input pad, because media-ctl will propagate formats from
> source to sink pads for you, so it's better to rely on the propagation,
> and set the CSI output pad format instead (full frame height at output pad):
>
> media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>

Steve,

Thanks - that makes sense.

I also noticed that if I setup one of the vdic pipelines first then
went back after a 'media-ctl -r' and setup the example that failed it
no longer failed. I'm thinking that this is because 'media-ctl -r'
make reset all the links but does not reset all the V4L2 formats on
pads?

>
> Final note: the imx.rst doc is technically correct even though it is
> showing full frame heights being configured at the pads, because it is
> expecting the adv7180 has accepted 'seq-bt'. But even the example given
> in that doc works for alternate field mode, because the pad heights are
> forced to the correct field height for alternate mode.
>

hmmm... I don't quite follow this statement. It sounds like the
example would only be correct if you were setting 'field:alternate'
but the example sets 'field:seq-bt' instead.

I wonder if you should add some verbiage explaining the difference in
format (resolution specifically) between the input and output pads
and/or change the example to set the output pad format so people don't
run into what I did trying to follow the example.

Tim
Tim Harvey Jan. 22, 2019, 7:51 p.m. UTC | #5
On Mon, Jan 21, 2019 at 12:24 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >
> > Hi Tim,
> >
> > On 1/15/19 1:58 PM, Tim Harvey wrote:
> > > On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> > >> Also add an example pipeline for unconverted capture with interweave
> > >> on SabreAuto.
> > >>
> > >> Cleanup some language in various places in the process.
> > >>
> > >> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> > >> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> > >> ---
> > >> Changes since v4:
> > >> - Make clear that it is IDMAC channel that does pixel reordering and
> > >>    interweave, not the CSI. Caught by Philipp Zabel.
> > >> Changes since v3:
> > >> - none.
> > >> Changes since v2:
> > >> - expand on idmac interweave behavior in CSI subdev.
> > >> - switch second SabreAuto pipeline example to PAL to give
> > >>    both NTSC and PAL examples.
> > >> - Cleanup some language in various places.
> > >> ---
> > >>   Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
> > >>   1 file changed, 66 insertions(+), 37 deletions(-)
> > >>
> > > <snip>
> > >>   Capture Pipelines
> > >>   -----------------
> > >> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
> > >>   parallel bus input on the internal video mux to IPU1 CSI0.
> > >>
> > >>   The following example configures a pipeline to capture from the ADV7180
> > >> -video decoder, assuming NTSC 720x480 input signals, with Motion
> > >> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
> > >> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
> > >> -entity at its output pad:
> > >> +video decoder, assuming NTSC 720x480 input signals, using simple
> > >> +interweave (unconverted and without motion compensation). The adv7180
> > >> +must output sequential or alternating fields (field type 'seq-bt' for
> > >> +NTSC, or 'alternate'):
> > >> +
> > >> +.. code-block:: none
> > >> +
> > >> +   # Setup links
> > >> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
> > >> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> > >> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> > >> +   # Configure pads
> > >> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> > >> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> > >> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> > >> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
> > >> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
> > >> +
> > >> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
> > >> +used to select any supported YUV pixelformat on /dev/video4.
> > >> +
> > > Hi Steve,
> > >
> > > I'm testing 4.20 with this patchset on top.
> > >
> > > I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
> > > the SabeAuto example above I can't get the simple interveave example
> > > to work:
> > >
> > > media-ctl -r # reset all links
> > > # Setup links (ADV7180 IPU1_CSI0)
> > > media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
> > > media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> > > media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
> > > # Configure pads
> > > media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> > > media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> > > media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
> >
> > This is the reason. The adv7180 is only allowing to configure alternate
> > field mode, and thus it reports the field height on the mbus, not the
> > full frame height. Imx deals with alternate field mode by capturing a
> > full frame, so the CSI entity sets the output pad height to double the
> > height.
> >
> > So the CSI input pad needs to be configured with the field height:
> >
> > media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"
> >
> > It should work for you after doing that. And better yet, don't bother
> > configuring the input pad, because media-ctl will propagate formats from
> > source to sink pads for you, so it's better to rely on the propagation,
> > and set the CSI output pad format instead (full frame height at output pad):
> >
> > media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> >
>
> Steve,
>
> Thanks - that makes sense.
>
> I also noticed that if I setup one of the vdic pipelines first then
> went back after a 'media-ctl -r' and setup the example that failed it
> no longer failed. I'm thinking that this is because 'media-ctl -r'
> make reset all the links but does not reset all the V4L2 formats on
> pads?
>
> >
> > Final note: the imx.rst doc is technically correct even though it is
> > showing full frame heights being configured at the pads, because it is
> > expecting the adv7180 has accepted 'seq-bt'. But even the example given
> > in that doc works for alternate field mode, because the pad heights are
> > forced to the correct field height for alternate mode.
> >
>
> hmmm... I don't quite follow this statement. It sounds like the
> example would only be correct if you were setting 'field:alternate'
> but the example sets 'field:seq-bt' instead.
>
> I wonder if you should add some verbiage explaining the difference in
> format (resolution specifically) between the input and output pads
> and/or change the example to set the output pad format so people don't
> run into what I did trying to follow the example.
>

Steve,

I'm able to link a sensor->mux->csi->vdic->ic_prp->ic_prpenc but not a
sensor->mux->csi->vdic->ic_prp->ic_prpvf pipeline:

- imx6q-gw54xx adv7180 2-0020 IPU2_CSI1 sensor->mux->csi->vdic->ic_prp->ic_prpvf
# sensor format
media-ctl --get-v4l2 '"adv7180 2-0020":0' #
fmt:UYVY8_2X8/720x240@1001/30000 field:alternate colorspace:smpte170m
# reset all links
media-ctl --reset
# 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 field:seq-bt]"
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]"
# capture device
media-ctl -e 'ipu2_ic_prpvf capture' # /dev/video3
# capture 1 frame
v4l2-ctl --device /dev/video3 --stream-mmap --stream-to=x.raw --stream-count=1
^^^ works

- imx6q-gw54xx adv7180 2-0020 IPU2_CSI1
sensor->mux->csi->vdic->ic_prp->ic_prpenc
# sensor format
media-ctl --get-v4l2 '"adv7180 2-0020":0' #
fmt:UYVY8_2X8/720x240@1001/30000 field:alternate colorspace:smpte170m
# reset all links
media-ctl --reset
# 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':1 -> 'ipu2_ic_prpenc':0[1]"
Unable to setup formats: Invalid argument (22)

using Linux 4.20 + the following patch series:
  - media: imx-csi: Input connections to CSI should be optional
  - imx-media: Fixes for interlaced capture
v4l-utils 1.16.1

See http://dev.gateworks.com/docs/linux/media/imx6q-gw54xx-media.png

My understanding is that the ic_prpenc and ic_prpvf entities are
identical and it looks like I'm using the right pads. I'm also seeing
the same on a board that uses ipu1_csi0 instead.

Any ideas?

Regards,

Tim
Steve Longerbeam Jan. 23, 2019, 12:08 a.m. UTC | #6
On 1/21/19 12:24 PM, Tim Harvey wrote:
> On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>> Hi Tim,
>>
>> On 1/15/19 1:58 PM, Tim Harvey wrote:
>>> On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>>> Also add an example pipeline for unconverted capture with interweave
>>>> on SabreAuto.
>>>>
>>>> Cleanup some language in various places in the process.
>>>>
>>>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>>>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>> ---
>>>> Changes since v4:
>>>> - Make clear that it is IDMAC channel that does pixel reordering and
>>>>     interweave, not the CSI. Caught by Philipp Zabel.
>>>> Changes since v3:
>>>> - none.
>>>> Changes since v2:
>>>> - expand on idmac interweave behavior in CSI subdev.
>>>> - switch second SabreAuto pipeline example to PAL to give
>>>>     both NTSC and PAL examples.
>>>> - Cleanup some language in various places.
>>>> ---
>>>>    Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
>>>>    1 file changed, 66 insertions(+), 37 deletions(-)
>>>>
>>> <snip>
>>>>    Capture Pipelines
>>>>    -----------------
>>>> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>>>>    parallel bus input on the internal video mux to IPU1 CSI0.
>>>>
>>>>    The following example configures a pipeline to capture from the ADV7180
>>>> -video decoder, assuming NTSC 720x480 input signals, with Motion
>>>> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
>>>> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
>>>> -entity at its output pad:
>>>> +video decoder, assuming NTSC 720x480 input signals, using simple
>>>> +interweave (unconverted and without motion compensation). The adv7180
>>>> +must output sequential or alternating fields (field type 'seq-bt' for
>>>> +NTSC, or 'alternate'):
>>>> +
>>>> +.. code-block:: none
>>>> +
>>>> +   # Setup links
>>>> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
>>>> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>>>> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
>>>> +   # Configure pads
>>>> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>>>> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>>>> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>>>> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
>>>> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
>>>> +
>>>> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
>>>> +used to select any supported YUV pixelformat on /dev/video4.
>>>> +
>>> Hi Steve,
>>>
>>> I'm testing 4.20 with this patchset on top.
>>>
>>> I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
>>> the SabeAuto example above I can't get the simple interveave example
>>> to work:
>>>
>>> media-ctl -r # reset all links
>>> # Setup links (ADV7180 IPU1_CSI0)
>>> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
>>> media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>>> media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
>>> # Configure pads
>>> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>>> media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
>> This is the reason. The adv7180 is only allowing to configure alternate
>> field mode, and thus it reports the field height on the mbus, not the
>> full frame height. Imx deals with alternate field mode by capturing a
>> full frame, so the CSI entity sets the output pad height to double the
>> height.
>>
>> So the CSI input pad needs to be configured with the field height:
>>
>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"
>>
>> It should work for you after doing that. And better yet, don't bother
>> configuring the input pad, because media-ctl will propagate formats from
>> source to sink pads for you, so it's better to rely on the propagation,
>> and set the CSI output pad format instead (full frame height at output pad):
>>
>> media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>>
> Steve,
>
> Thanks - that makes sense.
>
> I also noticed that if I setup one of the vdic pipelines first then
> went back after a 'media-ctl -r' and setup the example that failed it
> no longer failed. I'm thinking that this is because 'media-ctl -r'
> make reset all the links but does not reset all the V4L2 formats on
> pads?
>
>> Final note: the imx.rst doc is technically correct even though it is
>> showing full frame heights being configured at the pads, because it is
>> expecting the adv7180 has accepted 'seq-bt'. But even the example given
>> in that doc works for alternate field mode, because the pad heights are
>> forced to the correct field height for alternate mode.
>>
> hmmm... I don't quite follow this statement. It sounds like the
> example would only be correct if you were setting 'field:alternate'
> but the example sets 'field:seq-bt' instead.

The example is consistent for a sensor that sends seq-bt. Here is the 
example config from the imx.rst doc again, a (ntsc) height of 480 lines 
is correct for a seq-bt source:

    # Setup links
    media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
    media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
    media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
    # Configure pads
    media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
    media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
    media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
    # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
    v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt

> I wonder if you should add some verbiage explaining the difference in
> format (resolution specifically) between the input and output pads
> and/or change the example to set the output pad format so people don't
> run into what I did trying to follow the example.
>

Well, the example *is* setting the output pad format (media-ctl -V 
"ipu1_csi0:2 ...").

But I suppose wording could be added such as "this example assumes the 
sensor (adv7180) supports seq-bt".

Steve
Steve Longerbeam Jan. 23, 2019, 12:24 a.m. UTC | #7
On 1/22/19 11:51 AM, Tim Harvey wrote:
> On Mon, Jan 21, 2019 at 12:24 PM Tim Harvey <tharvey@gateworks.com> wrote:
>> On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>> Hi Tim,
>>>
>>> On 1/15/19 1:58 PM, Tim Harvey wrote:
>>>> On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>>>>> Also add an example pipeline for unconverted capture with interweave
>>>>> on SabreAuto.
>>>>>
>>>>> Cleanup some language in various places in the process.
>>>>>
>>>>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
>>>>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
>>>>> ---
>>>>> Changes since v4:
>>>>> - Make clear that it is IDMAC channel that does pixel reordering and
>>>>>     interweave, not the CSI. Caught by Philipp Zabel.
>>>>> Changes since v3:
>>>>> - none.
>>>>> Changes since v2:
>>>>> - expand on idmac interweave behavior in CSI subdev.
>>>>> - switch second SabreAuto pipeline example to PAL to give
>>>>>     both NTSC and PAL examples.
>>>>> - Cleanup some language in various places.
>>>>> ---
>>>>>    Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
>>>>>    1 file changed, 66 insertions(+), 37 deletions(-)
>>>>>
>>>> <snip>
>>>>>    Capture Pipelines
>>>>>    -----------------
>>>>> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
>>>>>    parallel bus input on the internal video mux to IPU1 CSI0.
>>>>>
>>>>>    The following example configures a pipeline to capture from the ADV7180
>>>>> -video decoder, assuming NTSC 720x480 input signals, with Motion
>>>>> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
>>>>> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
>>>>> -entity at its output pad:
>>>>> +video decoder, assuming NTSC 720x480 input signals, using simple
>>>>> +interweave (unconverted and without motion compensation). The adv7180
>>>>> +must output sequential or alternating fields (field type 'seq-bt' for
>>>>> +NTSC, or 'alternate'):
>>>>> +
>>>>> +.. code-block:: none
>>>>> +
>>>>> +   # Setup links
>>>>> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
>>>>> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>>>>> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
>>>>> +   # Configure pads
>>>>> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>>>>> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>>>>> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>>>>> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
>>>>> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
>>>>> +
>>>>> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
>>>>> +used to select any supported YUV pixelformat on /dev/video4.
>>>>> +
>>>> Hi Steve,
>>>>
>>>> I'm testing 4.20 with this patchset on top.
>>>>
>>>> I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
>>>> the SabeAuto example above I can't get the simple interveave example
>>>> to work:
>>>>
>>>> media-ctl -r # reset all links
>>>> # Setup links (ADV7180 IPU1_CSI0)
>>>> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
>>>> media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
>>>> media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
>>>> # Configure pads
>>>> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>>>> media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>>>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
>>> This is the reason. The adv7180 is only allowing to configure alternate
>>> field mode, and thus it reports the field height on the mbus, not the
>>> full frame height. Imx deals with alternate field mode by capturing a
>>> full frame, so the CSI entity sets the output pad height to double the
>>> height.
>>>
>>> So the CSI input pad needs to be configured with the field height:
>>>
>>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"
>>>
>>> It should work for you after doing that. And better yet, don't bother
>>> configuring the input pad, because media-ctl will propagate formats from
>>> source to sink pads for you, so it's better to rely on the propagation,
>>> and set the CSI output pad format instead (full frame height at output pad):
>>>
>>> media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>>>
>> Steve,
>>
>> Thanks - that makes sense.
>>
>> I also noticed that if I setup one of the vdic pipelines first then
>> went back after a 'media-ctl -r' and setup the example that failed it
>> no longer failed. I'm thinking that this is because 'media-ctl -r'
>> make reset all the links but does not reset all the V4L2 formats on
>> pads?
>>
>>> Final note: the imx.rst doc is technically correct even though it is
>>> showing full frame heights being configured at the pads, because it is
>>> expecting the adv7180 has accepted 'seq-bt'. But even the example given
>>> in that doc works for alternate field mode, because the pad heights are
>>> forced to the correct field height for alternate mode.
>>>
>> hmmm... I don't quite follow this statement. It sounds like the
>> example would only be correct if you were setting 'field:alternate'
>> but the example sets 'field:seq-bt' instead.
>>
>> I wonder if you should add some verbiage explaining the difference in
>> format (resolution specifically) between the input and output pads
>> and/or change the example to set the output pad format so people don't
>> run into what I did trying to follow the example.
>>
> Steve,
>
> I'm able to link a sensor->mux->csi->vdic->ic_prp->ic_prpenc but not a
> sensor->mux->csi->vdic->ic_prp->ic_prpvf pipeline:

I think you mean the reverse: can setup a vdic->ic_prp->ic_prpvf 
pipeline but not a vdic->ic_prp->ic_prpenc pipeline.

Anyway there is reason for that, see below.

> - imx6q-gw54xx adv7180 2-0020 IPU2_CSI1 sensor->mux->csi->vdic->ic_prp->ic_prpvf
> # sensor format
> media-ctl --get-v4l2 '"adv7180 2-0020":0' #
> fmt:UYVY8_2X8/720x240@1001/30000 field:alternate colorspace:smpte170m
> # reset all links
> media-ctl --reset
> # 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 field:seq-bt]"
> 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]"
> # capture device
> media-ctl -e 'ipu2_ic_prpvf capture' # /dev/video3
> # capture 1 frame
> v4l2-ctl --device /dev/video3 --stream-mmap --stream-to=x.raw --stream-count=1
> ^^^ works
>
> - imx6q-gw54xx adv7180 2-0020 IPU2_CSI1
> sensor->mux->csi->vdic->ic_prp->ic_prpenc
> # sensor format
> media-ctl --get-v4l2 '"adv7180 2-0020":0' #
> fmt:UYVY8_2X8/720x240@1001/30000 field:alternate colorspace:smpte170m
> # reset all links
> media-ctl --reset
> # 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':1 -> 'ipu2_ic_prpenc':0[1]"
> Unable to setup formats: Invalid argument (22)
>
> using Linux 4.20 + the following patch series:
>    - media: imx-csi: Input connections to CSI should be optional
>    - imx-media: Fixes for interlaced capture
> v4l-utils 1.16.1
>
> See http://dev.gateworks.com/docs/linux/media/imx6q-gw54xx-media.png
>
> My understanding is that the ic_prpenc and ic_prpvf entities are
> identical and it looks like I'm using the right pads. I'm also seeing
> the same on a board that uses ipu1_csi0 instead.
>
> Any ideas?

The VDIC only outputs to the IC PRPVF task, and not to the IC PRPENC 
task. That's just the way the IPU works. So the driver catches an 
attempt to create a vdic->ic_prp->ic_prpenc link and returns error.

There is some explanation of that in the imx.rst doc under description 
of ipuX_ic_prpvf entity:

"This entity is identical in operation to ipuX_ic_prpenc, with the same 
resizing and CSC operations and flip/rotation controls. It will receive 
and process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is 
receiving from ipuX_vdic."


Steve
Tim Harvey Jan. 23, 2019, 10:04 p.m. UTC | #8
On Tue, Jan 22, 2019 at 4:10 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>
>
> On 1/21/19 12:24 PM, Tim Harvey wrote:
> > On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >> Hi Tim,
> >>
> >> On 1/15/19 1:58 PM, Tim Harvey wrote:
> >>> On Wed, Jan 9, 2019 at 10:30 AM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> >>>> Also add an example pipeline for unconverted capture with interweave
> >>>> on SabreAuto.
> >>>>
> >>>> Cleanup some language in various places in the process.
> >>>>
> >>>> Signed-off-by: Steve Longerbeam <slongerbeam@gmail.com>
> >>>> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> >>>> ---
> >>>> Changes since v4:
> >>>> - Make clear that it is IDMAC channel that does pixel reordering and
> >>>>     interweave, not the CSI. Caught by Philipp Zabel.
> >>>> Changes since v3:
> >>>> - none.
> >>>> Changes since v2:
> >>>> - expand on idmac interweave behavior in CSI subdev.
> >>>> - switch second SabreAuto pipeline example to PAL to give
> >>>>     both NTSC and PAL examples.
> >>>> - Cleanup some language in various places.
> >>>> ---
> >>>>    Documentation/media/v4l-drivers/imx.rst | 103 +++++++++++++++---------
> >>>>    1 file changed, 66 insertions(+), 37 deletions(-)
> >>>>
> >>> <snip>
> >>>>    Capture Pipelines
> >>>>    -----------------
> >>>> @@ -516,10 +522,33 @@ On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
> >>>>    parallel bus input on the internal video mux to IPU1 CSI0.
> >>>>
> >>>>    The following example configures a pipeline to capture from the ADV7180
> >>>> -video decoder, assuming NTSC 720x480 input signals, with Motion
> >>>> -Compensated de-interlacing. Pad field types assume the adv7180 outputs
> >>>> -"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
> >>>> -entity at its output pad:
> >>>> +video decoder, assuming NTSC 720x480 input signals, using simple
> >>>> +interweave (unconverted and without motion compensation). The adv7180
> >>>> +must output sequential or alternating fields (field type 'seq-bt' for
> >>>> +NTSC, or 'alternate'):
> >>>> +
> >>>> +.. code-block:: none
> >>>> +
> >>>> +   # Setup links
> >>>> +   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
> >>>> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> >>>> +   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> >>>> +   # Configure pads
> >>>> +   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> >>>> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> >>>> +   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> >>>> +   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
> >>>> +   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
> >>>> +
> >>>> +Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
> >>>> +used to select any supported YUV pixelformat on /dev/video4.
> >>>> +
> >>> Hi Steve,
> >>>
> >>> I'm testing 4.20 with this patchset on top.
> >>>
> >>> I'm on a GW5104 which has an IMX6Q with the adv7180 on ipu1_csi0 like
> >>> the SabeAuto example above I can't get the simple interveave example
> >>> to work:
> >>>
> >>> media-ctl -r # reset all links
> >>> # Setup links (ADV7180 IPU1_CSI0)
> >>> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":1[1]'
> >>> media-ctl -l '"ipu1_csi0_mux":2 -> "ipu1_csi0":0[1]'
> >>> media-ctl -l '"ipu1_csi0":2 -> "ipu1_csi0 capture":0[1]' # /dev/video4
> >>> # Configure pads
> >>> media-ctl -V "'adv7180 2-0020':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> >>> media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
> >>> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x480]"
> >> This is the reason. The adv7180 is only allowing to configure alternate
> >> field mode, and thus it reports the field height on the mbus, not the
> >> full frame height. Imx deals with alternate field mode by capturing a
> >> full frame, so the CSI entity sets the output pad height to double the
> >> height.
> >>
> >> So the CSI input pad needs to be configured with the field height:
> >>
> >> media-ctl -V "'ipu1_csi0':0 [fmt:AYUV32/720x240]"
> >>
> >> It should work for you after doing that. And better yet, don't bother
> >> configuring the input pad, because media-ctl will propagate formats from
> >> source to sink pads for you, so it's better to rely on the propagation,
> >> and set the CSI output pad format instead (full frame height at output pad):
> >>
> >> media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
> >>
> > Steve,
> >
> > Thanks - that makes sense.
> >
> > I also noticed that if I setup one of the vdic pipelines first then
> > went back after a 'media-ctl -r' and setup the example that failed it
> > no longer failed. I'm thinking that this is because 'media-ctl -r'
> > make reset all the links but does not reset all the V4L2 formats on
> > pads?
> >
> >> Final note: the imx.rst doc is technically correct even though it is
> >> showing full frame heights being configured at the pads, because it is
> >> expecting the adv7180 has accepted 'seq-bt'. But even the example given
> >> in that doc works for alternate field mode, because the pad heights are
> >> forced to the correct field height for alternate mode.
> >>
> > hmmm... I don't quite follow this statement. It sounds like the
> > example would only be correct if you were setting 'field:alternate'
> > but the example sets 'field:seq-bt' instead.
>
> The example is consistent for a sensor that sends seq-bt. Here is the
> example config from the imx.rst doc again, a (ntsc) height of 480 lines
> is correct for a seq-bt source:
>
>     # Setup links
>     media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
>     media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>     media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
>     # Configure pads
>     media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>     media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
>     media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
>     # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
>     v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
>
> > I wonder if you should add some verbiage explaining the difference in
> > format (resolution specifically) between the input and output pads
> > and/or change the example to set the output pad format so people don't
> > run into what I did trying to follow the example.
> >
>
> Well, the example *is* setting the output pad format (media-ctl -V
> "ipu1_csi0:2 ...").
>
> But I suppose wording could be added such as "this example assumes the
> sensor (adv7180) supports seq-bt".
>

Steve,

Your absolutely right - my usage which set the input pad wasn't even
following your example (by accident) so your example is good and makes
sense.

Your explanation for the failure of linking vdic->ic_prp->ic_prpenc in
the previous message makes sense also.

I'm testing again with the tda1997x HDMI receiver which provides a
wide variety of input's to test IMX6 capture with (resolutions,
progressive vs interlaced, bt656 fmt vs yuvsmp bus format) and running
into some issues but I'll post them in a new thread.

This series looks good and does fix several interlaced capture issues.

Acked-by: Tim Harvey <tharvey@gateworks.com>

Tested on GW5104/GW5404 with adv7180

Tested-by: Tim Harvey <tharvey@gateowrks.com>

Thanks,

Tim
diff mbox series

Patch

diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
index 6922dde4a82b..9314af00d067 100644
--- a/Documentation/media/v4l-drivers/imx.rst
+++ b/Documentation/media/v4l-drivers/imx.rst
@@ -24,8 +24,8 @@  memory. Various dedicated DMA channels exist for both video capture and
 display paths. During transfer, the IDMAC is also capable of vertical
 image flip, 8x8 block transfer (see IRT description), pixel component
 re-ordering (for example UYVY to YUYV) within the same colorspace, and
-even packed <--> planar conversion. It can also perform a simple
-de-interlacing by interleaving even and odd lines during transfer
+packed <--> planar conversion. The IDMAC can also perform a simple
+de-interlacing by interweaving even and odd lines during transfer
 (without motion compensation which requires the VDIC).
 
 The CSI is the backend capture unit that interfaces directly with
@@ -175,15 +175,21 @@  via the SMFC and an IDMAC channel, bypassing IC pre-processing. This
 source pad is routed to a capture device node, with a node name of the
 format "ipuX_csiY capture".
 
-Note that since the IDMAC source pad makes use of an IDMAC channel, it
-can do pixel reordering within the same colorspace. For example, the
-sink pad can take UYVY2X8, but the IDMAC source pad can output YUYV2X8.
-If the sink pad is receiving YUV, the output at the capture device can
-also be converted to a planar YUV format such as YUV420.
-
-It will also perform simple de-interlace without motion compensation,
-which is activated if the sink pad's field type is an interlaced type,
-and the IDMAC source pad field type is set to none.
+Note that since the IDMAC source pad makes use of an IDMAC channel,
+pixel reordering within the same colorspace can be carried out by the
+IDMAC channel. For example, if the CSI sink pad is receiving in UYVY
+order, the capture device linked to the IDMAC source pad can capture
+in YUYV order. Also, if the CSI sink pad is receiving a packed YUV
+format, the capture device can capture a planar YUV format such as
+YUV420.
+
+The IDMAC channel at the IDMAC source pad also supports simple
+interweave without motion compensation, which is activated if the source
+pad's field type is sequential top-bottom or bottom-top, and the
+requested capture interface field type is set to interlaced (t-b, b-t,
+or unqualified interlaced). The capture interface will enforce the same
+field order as the source pad field order (interlaced-bt if source pad
+is seq-bt, interlaced-tb if source pad is seq-tb).
 
 This subdev can generate the following event when enabling the second
 IDMAC source pad:
@@ -325,14 +331,14 @@  ipuX_vdic
 
 The VDIC carries out motion compensated de-interlacing, with three
 motion compensation modes: low, medium, and high motion. The mode is
-specified with the menu control V4L2_CID_DEINTERLACING_MODE. It has
-two sink pads and a single source pad.
+specified with the menu control V4L2_CID_DEINTERLACING_MODE. The VDIC
+has two sink pads and a single source pad.
 
 The direct sink pad receives from an ipuX_csiY direct pad. With this
 link the VDIC can only operate in high motion mode.
 
 When the IDMAC sink pad is activated, it receives from an output
-or mem2mem device node. With this pipeline, it can also operate
+or mem2mem device node. With this pipeline, the VDIC can also operate
 in low and medium modes, because these modes require receiving
 frames from memory buffers. Note that an output or mem2mem device
 is not implemented yet, so this sink pad currently has no links.
@@ -345,8 +351,8 @@  ipuX_ic_prp
 This is the IC pre-processing entity. It acts as a router, routing
 data from its sink pad to one or both of its source pads.
 
-It has a single sink pad. The sink pad can receive from the ipuX_csiY
-direct pad, or from ipuX_vdic.
+This entity has a single sink pad. The sink pad can receive from the
+ipuX_csiY direct pad, or from ipuX_vdic.
 
 This entity has two source pads. One source pad routes to the
 pre-process encode task entity (ipuX_ic_prpenc), the other to the
@@ -369,8 +375,8 @@  color-space conversion, resizing (downscaling and upscaling),
 horizontal and vertical flip, and 90/270 degree rotation. Flip
 and rotation are provided via standard V4L2 controls.
 
-Like the ipuX_csiY IDMAC source, it can also perform simple de-interlace
-without motion compensation, and pixel reordering.
+Like the ipuX_csiY IDMAC source, this entity also supports simple
+de-interlace without motion compensation, and pixel reordering.
 
 ipuX_ic_prpvf
 -------------
@@ -380,18 +386,18 @@  pad from ipuX_ic_prp, and a single source pad. The source pad is routed
 to a capture device node, with a node name of the format
 "ipuX_ic_prpvf capture".
 
-It is identical in operation to ipuX_ic_prpenc, with the same resizing
-and CSC operations and flip/rotation controls. It will receive and
-process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is
+This entity is identical in operation to ipuX_ic_prpenc, with the same
+resizing and CSC operations and flip/rotation controls. It will receive
+and process de-interlaced frames from the ipuX_vdic if ipuX_ic_prp is
 receiving from ipuX_vdic.
 
-Like the ipuX_csiY IDMAC source, it can perform simple de-interlace
-without motion compensation. However, note that if the ipuX_vdic is
-included in the pipeline (ipuX_ic_prp is receiving from ipuX_vdic),
-it's not possible to use simple de-interlace in ipuX_ic_prpvf, since
-the ipuX_vdic has already carried out de-interlacing (with motion
-compensation) and therefore the field type output from ipuX_ic_prp can
-only be none.
+Like the ipuX_csiY IDMAC source, this entity supports simple
+interweaving without motion compensation. However, note that if the
+ipuX_vdic is included in the pipeline (ipuX_ic_prp is receiving from
+ipuX_vdic), it's not possible to use interweave in ipuX_ic_prpvf,
+since the ipuX_vdic has already carried out de-interlacing (with
+motion compensation) and therefore the field type output from
+ipuX_vdic can only be none (progressive).
 
 Capture Pipelines
 -----------------
@@ -516,10 +522,33 @@  On the SabreAuto, an on-board ADV7180 SD decoder is connected to the
 parallel bus input on the internal video mux to IPU1 CSI0.
 
 The following example configures a pipeline to capture from the ADV7180
-video decoder, assuming NTSC 720x480 input signals, with Motion
-Compensated de-interlacing. Pad field types assume the adv7180 outputs
-"interlaced". $outputfmt can be any format supported by the ipu1_ic_prpvf
-entity at its output pad:
+video decoder, assuming NTSC 720x480 input signals, using simple
+interweave (unconverted and without motion compensation). The adv7180
+must output sequential or alternating fields (field type 'seq-bt' for
+NTSC, or 'alternate'):
+
+.. code-block:: none
+
+   # Setup links
+   media-ctl -l "'adv7180 3-0021':0 -> 'ipu1_csi0_mux':1[1]"
+   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
+   media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
+   # Configure pads
+   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
+   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480]"
+   media-ctl -V "'ipu1_csi0':2 [fmt:AYUV32/720x480]"
+   # Configure "ipu1_csi0 capture" interface (assumed at /dev/video4)
+   v4l2-ctl -d4 --set-fmt-video=field=interlaced_bt
+
+Streaming can then begin on /dev/video4. The v4l2-ctl tool can also be
+used to select any supported YUV pixelformat on /dev/video4.
+
+This example configures a pipeline to capture from the ADV7180
+video decoder, assuming PAL 720x576 input signals, with Motion
+Compensated de-interlacing. The adv7180 must output sequential or
+alternating fields (field type 'seq-tb' for PAL, or 'alternate').
+$outputfmt can be any format supported by the ipu1_ic_prpvf entity
+at its output pad:
 
 .. code-block:: none
 
@@ -531,11 +560,11 @@  entity at its output pad:
    media-ctl -l "'ipu1_ic_prp':2 -> 'ipu1_ic_prpvf':0[1]"
    media-ctl -l "'ipu1_ic_prpvf':1 -> 'ipu1_ic_prpvf capture':0[1]"
    # Configure pads
-   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x480]"
-   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x480 field:interlaced]"
-   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x480 field:interlaced]"
-   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x480 field:none]"
-   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x480 field:none]"
+   media-ctl -V "'adv7180 3-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
+   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
+   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/720x576]"
+   media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/720x576 field:none]"
+   media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/720x576 field:none]"
    media-ctl -V "'ipu1_ic_prpvf':1 [fmt:$outputfmt field:none]"
 
 Streaming can then begin on the capture device node at