linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve Longerbeam <slongerbeam@gmail.com>
To: Tim Harvey <tharvey@gateworks.com>
Cc: linux-media <linux-media@vger.kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v8 11/11] media: imx.rst: Update doc to reflect fixes to interlaced capture
Date: Tue, 15 Jan 2019 15:54:54 -0800	[thread overview]
Message-ID: <6b4c3fb1-929b-8894-e2f9-aca2f392f0e5@gmail.com> (raw)
In-Reply-To: <CAJ+vNU1r86n1=9gKDw-bTO0sWJL7NMjZcdKMQO23a+WOR1H9tw@mail.gmail.com>

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


  reply	other threads:[~2019-01-15 23:55 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 18:30 [PATCH v8 00/11] imx-media: Fixes for interlaced capture Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 01/11] media: videodev2.h: Add more field helper macros Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 02/11] gpu: ipu-csi: Swap fields according to input/output field types Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 03/11] gpu: ipu-v3: Add planar support to interlaced scan Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 04/11] media: imx: Fix field negotiation Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 05/11] media: imx-csi: Double crop height for alternate fields at sink Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 06/11] media: imx: interweave and odd-chroma-row skip are incompatible Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 07/11] media: imx-csi: Allow skipping odd chroma rows for YVU420 Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 08/11] media: imx: vdic: rely on VDIC for correct field order Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 09/11] media: imx-csi: Move crop/compose reset after filling default mbus fields Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 10/11] media: imx: Allow interweave with top/bottom lines swapped Steve Longerbeam
2019-01-09 18:30 ` [PATCH v8 11/11] media: imx.rst: Update doc to reflect fixes to interlaced capture Steve Longerbeam
2019-01-15 21:58   ` Tim Harvey
2019-01-15 23:54     ` Steve Longerbeam [this message]
2019-01-15 23:59       ` Steve Longerbeam
2019-01-21 20:24       ` Tim Harvey
2019-01-22 19:51         ` Tim Harvey
2019-01-23  0:24           ` Steve Longerbeam
2019-01-23  0:08         ` Steve Longerbeam
2019-01-23 22:04           ` Tim Harvey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6b4c3fb1-929b-8894-e2f9-aca2f392f0e5@gmail.com \
    --to=slongerbeam@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=tharvey@gateworks.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).