Hi Tim,


On 01/12/2017 01:13 PM, Tim Harvey wrote:
>
>>> Now that your driver is hooking into the current media framework, I'm
>>> not at all clear on how to link and configure the media entities.
>>
>> It's all documented at Documentation/media/v4l-drivers/imx.rst.
>> Follow the SabreAuto pipeline setup example.
>>
> ah yes... it helps to read your patches! You did a great job on the
> documentation.
>
> Regarding the The ipu1_csi0_mux/ipu2_csi1_mux entities which have 1
> source and 2 sinks (which makes sense for a mux) how do you know which
> sink pad you should use (in your adv7180 example you use the 2nd sink
> pad vs the first)?

The adv7180 can only go to the parallel input pad (ipu1_csi0_mux:1
on quad). The other input pads select from the mipi csi-2 receiver virtual
channels.

Have you generated a dot graph? It makes it much easier to
visualize:

# media-ctl --print-dot > graph.dot

then on your host:

% dot -Tpng graph.dot > graph.png


>
> As my hardware is the same as the SabreAuto except that my adv7180 is
> on i2c-2@0x20 I follow your example from
> Documentation/media/v4l-drivers/imx.rst:
>
> # Setup links
> 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":1 -> "ipu1_smfc0":0[1]'
> media-ctl -l '"ipu1_smfc0":1 -> "ipu1_ic_prpvf":0[1]'
> media-ctl -l '"ipu1_ic_prpvf":1 -> "camif0":0[1]'
> media-ctl -l '"camif0":1 -> "camif0 devnode":0[1]'
>
> # Configure pads
> media-ctl -V "\"adv7180 2-0020\":0 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_csi0_mux\":1 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_csi0_mux\":2 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_csi0\":0 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_csi0\":1 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_smfc0\":0 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_smfc0\":1 [fmt:UYVY2X8/720x480]"
> media-ctl -V "\"ipu1_ic_prpvf\":0 [fmt:UYVY2X8/720x480]"
> # pad field types for camif can be any format prpvf supports
> export outputfmt="UYVY2X8/720x480"
> media-ctl -V "\"ipu1_ic_prpvf\":1 [fmt:$outputfmt]"
> media-ctl -V "\"camif0\":0 [fmt:$outputfmt]"
> media-ctl -V "\"camif0\":1 [fmt:$outputfmt]"
>
> # select AIN1
> v4l2-ctl -d0 -i0
> Video input set to 0 (ADV7180 Composite on Ain1: ok)
> v4l2-ctl -d0 --set-fmt-video=width=720,height=480,pixelformat=UYVY
> # capture a single raw frame
> v4l2-ctl -d0 --stream-mmap --stream-to=/x.raw --stream-count=1
> [ 2092.056394] camif0: pipeline_set_stream failed with -32
> VIDIOC_STREAMON: failed: Broken pipe
>
> Enabling debug in drivers/media/media-entity.c I see:
> [   38.870087] imx-media soc:media@0: link validation failed for
> "ipu1_smfc0":1 -> "ipu1_ic_prpvf":0, error -32
>
> Looking at ipu1_smfc0 and ipu1_ic_prpvf with media-ctl I see:
> - entity 12: ipu1_ic_prpvf (2 pads, 8 links)
>               type V4L2 subdev subtype Unknown flags 0
>               device node name /dev/v4l-subdev3
>          pad0: Sink
>                  [fmt:UYVY2X8/720x480 field:alternate]
>                  <- "ipu1_csi0":1 []
>                  <- "ipu1_csi1":1 []
>                  <- "ipu1_smfc0":1 [ENABLED]
>                  <- "ipu1_smfc1":1 []
>          pad1: Source
>                  [fmt:UYVY2X8/720x480 field:none]
>                  -> "camif0":0 [ENABLED]
>                  -> "camif1":0 []
>                  -> "ipu1_ic_pp0":0 []
>                  -> "ipu1_ic_pp1":0 []
>
> - entity 45: ipu1_smfc0 (2 pads, 5 links)
>               type V4L2 subdev subtype Unknown flags 0
>               device node name /dev/v4l-subdev14
>          pad0: Sink
>                  [fmt:UYVY2X8/720x480]
>                  <- "ipu1_csi0":1 [ENABLED]
>          pad1: Source
>                  [fmt:UYVY2X8/720x480]
>                  -> "ipu1_ic_prpvf":0 [ENABLED]
>                  -> "ipu1_ic_pp0":0 []
>                  -> "camif0":0 []
>                  -> "camif1":0 []
>
> Any ideas what is going wrong here? Seems like its perhaps a field
> type mismatch.

Yes, exactly, you'll need to set the field types on every pad in your
pipeline.

>   Is my outputfmt incorrect perhaps? I likely have
> misunderstood the pad type comments in your documentation.

Attached is an update doc (from branch imx-media-staging-md-v7 on my fork).
I recently upgraded my v4l-utils package and media-ctl now supports 
specifying
the field type in the pad format strings. If you don't have the latest 
v4l-utils, it's
fairly straightforward to cross-build.

>
>>
>>> <snip>
>>>
>>>
>>>
>>> Additionally I've found that on an IMX6S/IMX6DL we crash while
>>> registering the media-ic subdev's:
> <snip>
>> Yep, I only have quad boards here so I haven't gotten around to
>> testing on S/DL.
>>
>> But it looks like I forgot to clear out the csi subdev pointer array before
>> passing it to imx_media_of_parse(). I think that might explain the OOPS
>> above. Try this patch:
>>
>> diff --git a/drivers/staging/media/imx/imx-media-dev.c
>> b/drivers/staging/media/imx/imx-media-dev.c
>> index 357654d..0cf2d61 100644
>> --- a/drivers/staging/media/imx/imx-media-dev.c
>> +++ b/drivers/staging/media/imx/imx-media-dev.c
>> @@ -379,7 +379,7 @@ static int imx_media_probe(struct platform_device *pdev)
>>   {
>>          struct device *dev = &pdev->dev;
>>          struct device_node *node = dev->of_node;
>> -       struct imx_media_subdev *csi[4];
>> +       struct imx_media_subdev *csi[4] = {0};
>>          struct imx_media_dev *imxmd;
>>          int ret;
>>
> This does resolves the crash on S/DL.

Cool thanks for verifying, I've applied this to the imx-media-staging-md-v7
branch.

>
> I do notice that the ipu1_csi*_mux entities on the S/DL have 3 more
> sink pads compared to the D/Q which is from the additional ports
> defined in the GPR nodes you add for mipi_vc1/vc2/vc3. Are there
> really 3 more MIPI virtual channels on the S/DL vs the D/Q?

Well, same number of virtual channels on quad vs S/DL. It's
just that the video mux on S/DL can select from all 4 virtual
channels, whereas quad's mux'es only select either vc0 or vc3.

>
> I get the same results on the S/DL as I do on D/Q as long as I adjust
> the links to compensate for these additional sinks:
> media-ctl -l '"adv7180 2-0020":0 -> "ipu1_csi0_mux":4[1]'  # pad4
> media-ctl -l '"ipu1_csi0_mux":5 -> "ipu1_csi0":0[1]' # pad5
> ...
>
> This means link configuration must differ depending on S/DL vs D/Q
> which is a bummer but I suppose this is the harsh reality as for
> boards that use the EIM pads for IPU's they also will be using IPU2
> for IMX6D/Q and IPU1 for IMX6S/DL.

yeah, the links necessarily must be different between quad and S/DL.

Steve