Message-ID: <CAJ+vNU2827H8C3PN=v++XRjd0LP6Uf1KzMAs0bFTgbX93v7atg@mail.gmail.com>
Date:   Mon, 21 Jan 2019 12:24:40 -0800
From:   Tim Harvey <tharvey@...eworks.com>
To:     Steve Longerbeam <slongerbeam@...il.com>
Cc:     linux-media <linux-media@...r.kernel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v8 11/11] media: imx.rst: Update doc to reflect fixes to
 interlaced capture

On Tue, Jan 15, 2019 at 3:54 PM Steve Longerbeam <slongerbeam@...il.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@...il.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@...il.com>
> >> Reviewed-by: Philipp Zabel <p.zabel@...gutronix.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

Powered by blists - more mailing lists