[PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

In kernel 5.3.x the I2C bus that the adv7180 is under has changed from
bus 3 to 4 and the ipu_csi0_mux numbers has also changed as shown by
the kernel log below: 

[    5.159423] imx-media: ipu1_csi0_mux:5 -> ipu1_csi0:0
[    5.164618] imx-media: ipu1_csi1_mux:5 -> ipu1_csi1:0
[    5.169953] imx-media: adv7180 4-0021:0 -> ipu1_csi0_mux:4

Update the instructions accordingly.

Also, in the last line pass the fmt field explicitly as done in the
previous lines.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 Documentation/media/v4l-drivers/imx.rst | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
index 1d7eb8c7bd5c..75430cd2f222 100644
--- a/Documentation/media/v4l-drivers/imx.rst
+++ b/Documentation/media/v4l-drivers/imx.rst
@@ -530,12 +530,12 @@ 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
+   media-ctl -l "'ipu1_csi0_mux':5 -> '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 "'adv7180 4-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
+   media-ctl -V "'ipu1_csi0_mux':5 [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
@@ -553,19 +553,19 @@ at its output pad:
 .. 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
+   media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]"
    media-ctl -l "'ipu1_csi0':1 -> 'ipu1_vdic':0[1]"
    media-ctl -l "'ipu1_vdic':2 -> 'ipu1_ic_prp':0[1]"
    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/720x576 field:seq-tb]"
-   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
+   media-ctl -V "'adv7180 4-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
+   media-ctl -V "'ipu1_csi0_mux':5 [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]"
+   media-ctl -V "'ipu1_ic_prpvf':1 [fmt:AYUV32/720x576 field:none]"
 
 Streaming can then begin on the capture device node at
 "ipu1_ic_prpvf capture". The v4l2-ctl tool can be used to select any
-- 
2.17.1

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

Hi Fabio,

On 10/10/19 8:44 AM, Fabio Estevam wrote:
> In kernel 5.3.x the I2C bus that the adv7180 is under has changed from
> bus 3 to 4 and the ipu_csi0_mux numbers has also changed as shown by
> the kernel log below:
>
> [    5.159423] imx-media: ipu1_csi0_mux:5 -> ipu1_csi0:0
> [    5.164618] imx-media: ipu1_csi1_mux:5 -> ipu1_csi1:0
> [    5.169953] imx-media: adv7180 4-0021:0 -> ipu1_csi0_mux:4
>
> Update the instructions accordingly.

I just booted the imx6q SabreAuto with 5.4.0-rc1 (and media-tree HEAD 
3ff3a712a9ea), I am getting:

arm:~ % uname -r
5.4.0-rc1-13672-g3ff3a712a9ea
arm:~ % dmesg | grep 'ipu[12]_csi[01]_mux'
[   13.042519] imx-media: ipu1_csi0_mux:2 -> ipu1_csi0:0
[   13.042590] imx-media: ipu2_csi1_mux:2 -> ipu2_csi1:0
[   13.042656] imx-media: adv7180 3-0021:0 -> ipu1_csi0_mux:1
arm:~ % dmesg | grep adv7180
[   12.905157] adv7180 3-0021: chip found @ 0x21 (i2c-2-mux (chan_id 1))
[   13.042656] imx-media: adv7180 3-0021:0 -> ipu1_csi0_mux:1


So the adv7180 i2c-bus and pad numbers have not changed. The adv7180 is 
still on bus 3, and the output pad from the IPU-CSI video muxes are 
still pad 2. The video muxes have no such pad 5 in fact. I'd be curious 
in learning how you came about that pad number.

> Also, in the last line pass the fmt field explicitly as done in the
> previous lines.

Thanks for catching that, it was left over from a pipeline configuration 
script of mine.

Steve

>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>   Documentation/media/v4l-drivers/imx.rst | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
> index 1d7eb8c7bd5c..75430cd2f222 100644
> --- a/Documentation/media/v4l-drivers/imx.rst
> +++ b/Documentation/media/v4l-drivers/imx.rst
> @@ -530,12 +530,12 @@ 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
> +   media-ctl -l "'ipu1_csi0_mux':5 -> '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 "'adv7180 4-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
> +   media-ctl -V "'ipu1_csi0_mux':5 [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
> @@ -553,19 +553,19 @@ at its output pad:
>   .. 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
> +   media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]"
>      media-ctl -l "'ipu1_csi0':1 -> 'ipu1_vdic':0[1]"
>      media-ctl -l "'ipu1_vdic':2 -> 'ipu1_ic_prp':0[1]"
>      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/720x576 field:seq-tb]"
> -   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
> +   media-ctl -V "'adv7180 4-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
> +   media-ctl -V "'ipu1_csi0_mux':5 [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]"
> +   media-ctl -V "'ipu1_ic_prpvf':1 [fmt:AYUV32/720x576 field:none]"
>   
>   Streaming can then begin on the capture device node at
>   "ipu1_ic_prpvf capture". The v4l2-ctl tool can be used to select any

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

On 10/12/19 1:14 PM, Steve Longerbeam wrote:
> Hi Fabio,
>
> On 10/10/19 8:44 AM, Fabio Estevam wrote:
>> In kernel 5.3.x the I2C bus that the adv7180 is under has changed from
>> bus 3 to 4 and the ipu_csi0_mux numbers has also changed as shown by
>> the kernel log below:
>>
>> [    5.159423] imx-media: ipu1_csi0_mux:5 -> ipu1_csi0:0
>> [    5.164618] imx-media: ipu1_csi1_mux:5 -> ipu1_csi1:0
>> [    5.169953] imx-media: adv7180 4-0021:0 -> ipu1_csi0_mux:4
>>
>> Update the instructions accordingly.
>
> I just booted the imx6q SabreAuto with 5.4.0-rc1 (and media-tree HEAD 
> 3ff3a712a9ea), I am getting:
>
> arm:~ % uname -r
> 5.4.0-rc1-13672-g3ff3a712a9ea
> arm:~ % dmesg | grep 'ipu[12]_csi[01]_mux'
> [   13.042519] imx-media: ipu1_csi0_mux:2 -> ipu1_csi0:0
> [   13.042590] imx-media: ipu2_csi1_mux:2 -> ipu2_csi1:0
> [   13.042656] imx-media: adv7180 3-0021:0 -> ipu1_csi0_mux:1
> arm:~ % dmesg | grep adv7180
> [   12.905157] adv7180 3-0021: chip found @ 0x21 (i2c-2-mux (chan_id 1))
> [   13.042656] imx-media: adv7180 3-0021:0 -> ipu1_csi0_mux:1
>
>
> So the adv7180 i2c-bus and pad numbers have not changed. The adv7180 
> is still on bus 3, and the output pad from the IPU-CSI video muxes are 
> still pad 2. The video muxes have no such pad 5 in fact. I'd be 
> curious in learning how you came about that pad number.

Ah, now I remember. You are using the imx6dl sabreauto, it's IPU-CSI 
video muxes have five input pads for each of the four MIPI CSI-2 virtual 
channels (pads 0 - 3) and one parallel input (pad 4). The output mux pad 
is pad 5.

But the doc should clarify which SabreAuto corresponds to the given 
example pipeline config. Can you send a v2 of this patch that clarifies 
the config corresponds to the imx6 quad SabreAuto. If you like you could 
also include the analogous config for the imx6dl.

Steve

>
>> Also, in the last line pass the fmt field explicitly as done in the
>> previous lines.
>
> Thanks for catching that, it was left over from a pipeline 
> configuration script of mine.
>
> Steve
>
>>
>> Signed-off-by: Fabio Estevam <festevam@gmail.com>
>> ---
>>   Documentation/media/v4l-drivers/imx.rst | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/Documentation/media/v4l-drivers/imx.rst 
>> b/Documentation/media/v4l-drivers/imx.rst
>> index 1d7eb8c7bd5c..75430cd2f222 100644
>> --- a/Documentation/media/v4l-drivers/imx.rst
>> +++ b/Documentation/media/v4l-drivers/imx.rst
>> @@ -530,12 +530,12 @@ 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
>> +   media-ctl -l "'ipu1_csi0_mux':5 -> '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 "'adv7180 4-0021':0 [fmt:UYVY2X8/720x480 field:seq-bt]"
>> +   media-ctl -V "'ipu1_csi0_mux':5 [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
>> @@ -553,19 +553,19 @@ at its output pad:
>>   .. 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 "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
>> +   media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]"
>>      media-ctl -l "'ipu1_csi0':1 -> 'ipu1_vdic':0[1]"
>>      media-ctl -l "'ipu1_vdic':2 -> 'ipu1_ic_prp':0[1]"
>>      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/720x576 field:seq-tb]"
>> -   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/720x576]"
>> +   media-ctl -V "'adv7180 4-0021':0 [fmt:UYVY2X8/720x576 field:seq-tb]"
>> +   media-ctl -V "'ipu1_csi0_mux':5 [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]"
>> +   media-ctl -V "'ipu1_ic_prpvf':1 [fmt:AYUV32/720x576 field:none]"
>>     Streaming can then begin on the capture device node at
>>   "ipu1_ic_prpvf capture". The v4l2-ctl tool can be used to select any
>

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Steve,

On Sat, Oct 12, 2019 at 5:24 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> Ah, now I remember. You are using the imx6dl sabreauto, it's IPU-CSI

Yes, correct. I am using the imx6dl-sabreauto.

> video muxes have five input pads for each of the four MIPI CSI-2 virtual
> channels (pads 0 - 3) and one parallel input (pad 4). The output mux pad
> is pad 5.
>
> But the doc should clarify which SabreAuto corresponds to the given
> example pipeline config. Can you send a v2 of this patch that clarifies
> the config corresponds to the imx6 quad SabreAuto. If you like you could
> also include the analogous config for the imx6dl.

Good idea. Will send a v2 that clarifies the example pipeline for each
sabreauto variant.

Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Tim Harvey]

On Sat, Oct 12, 2019 at 2:19 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Steve,
>
> On Sat, Oct 12, 2019 at 5:24 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
> > Ah, now I remember. You are using the imx6dl sabreauto, it's IPU-CSI
>
> Yes, correct. I am using the imx6dl-sabreauto.
>
> > video muxes have five input pads for each of the four MIPI CSI-2 virtual
> > channels (pads 0 - 3) and one parallel input (pad 4). The output mux pad
> > is pad 5.
> >
> > But the doc should clarify which SabreAuto corresponds to the given
> > example pipeline config. Can you send a v2 of this patch that clarifies
> > the config corresponds to the imx6 quad SabreAuto. If you like you could
> > also include the analogous config for the imx6dl.
>
> Good idea. Will send a v2 that clarifies the example pipeline for each
> sabreauto variant.
>
> Thanks

Fabio,

I assume your seeing the same rolling video issue on capture unless
you discard the first few 'corrupt' frames? I'm still wondering how we
can address this properly upstream.

Tim

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Tim,

On Tue, Oct 15, 2019 at 12:49 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> I assume your seeing the same rolling video issue on capture unless
> you discard the first few 'corrupt' frames? I'm still wondering how we
> can address this properly upstream.

On i.MX53 I still get the rolling video even if I discard the few
corrupt frames.

I tested the same i.MX53 hardware with a vendor 2.6.35 kernel and it
captures correctly, no scrolling at all.

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Tim Harvey]

On Tue, Oct 15, 2019 at 8:53 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Tue, Oct 15, 2019 at 12:49 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> > Fabio,
> >
> > I assume your seeing the same rolling video issue on capture unless
> > you discard the first few 'corrupt' frames? I'm still wondering how we
> > can address this properly upstream.
>
> On i.MX53 I still get the rolling video even if I discard the few
> corrupt frames.
>
> I tested the same i.MX53 hardware with a vendor 2.6.35 kernel and it
> captures correctly, no scrolling at all.

Right, I understand there is something else going on with the i.MX53
but what about the i.MX6 testing related to these patches?

Regarding the i.MX53 as you have a kernel that does work you could
start looking for i2c register differences and csi config differences
between the two kernels. I discovered an issue with the adv7280 by
comparing i2c register dumps.

Tim

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Tim,

On Tue, Oct 15, 2019 at 1:07 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> Right, I understand there is something else going on with the i.MX53
> but what about the i.MX6 testing related to these patches?

I tested it on a imx6 sabreauto board with 5.3.x kernel plus Steve's
patch that discard the 10 initial corrupted frames and I do not get
rolling video.

I do get rolling video if I remove/insert the cable though.

> Regarding the i.MX53 as you have a kernel that does work you could
> start looking for i2c register differences and csi config differences
> between the two kernels. I discovered an issue with the adv7280 by
> comparing i2c register dumps.

Yes, I am in the process of comparing the two kernels.

Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Tim Harvey]

On Tue, Oct 15, 2019 at 9:16 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Tue, Oct 15, 2019 at 1:07 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > Right, I understand there is something else going on with the i.MX53
> > but what about the i.MX6 testing related to these patches?
>
> I tested it on a imx6 sabreauto board with 5.3.x kernel plus Steve's
> patch that discard the 10 initial corrupted frames and I do not get
> rolling video.
>
> I do get rolling video if I remove/insert the cable though.

Right, because this re-creates the initial issue. Upon any signal lock
you would need to throw away the first few frames. I wish I knew the
proper way to deal with this.

Tim

>
> > Regarding the i.MX53 as you have a kernel that does work you could
> > start looking for i2c register differences and csi config differences
> > between the two kernels. I discovered an issue with the adv7280 by
> > comparing i2c register dumps.
>
> Yes, I am in the process of comparing the two kernels.
>
> Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

On Tue, Oct 15, 2019 at 1:33 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Right, because this re-creates the initial issue. Upon any signal lock
> you would need to throw away the first few frames. I wish I knew the
> proper way to deal with this.

I thought this was handled by drivers/staging/media/imx/imx-media-fim.c

On imx6-sabreauto we do have the input capture pins configured in the
pinctrl entries, but I don't see any driver handling it.

Also, drivers/staging/media/imx/imx-media-fim.c driver has  a #ifdef
CONFIG_IMX_GPT_ICAP to deal with the input capture case, but such
config symbol does not exist in mainline.

It seems we still need the input capture support to be able to
measuring frame intervals.

Maybe Steve can shed some light on this?

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

On 10/15/19 9:42 AM, Fabio Estevam wrote:
> On Tue, Oct 15, 2019 at 1:33 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
>> Right, because this re-creates the initial issue. Upon any signal lock
>> you would need to throw away the first few frames. I wish I knew the
>> proper way to deal with this.
> I thought this was handled by drivers/staging/media/imx/imx-media-fim.c
>
> On imx6-sabreauto we do have the input capture pins configured in the
> pinctrl entries, but I don't see any driver handling it.
>
> Also, drivers/staging/media/imx/imx-media-fim.c driver has  a #ifdef
> CONFIG_IMX_GPT_ICAP to deal with the input capture case, but such
> config symbol does not exist in mainline.
>
> It seems we still need the input capture support to be able to
> measuring frame intervals.
>
> Maybe Steve can shed some light on this?

I submitted the ICAP driver patch quite a while ago, it was ~2 yrs ago I 
think. Can't seem to find the link unfortunately.

I'll work on updating the driver and retesting, and try resubmitting again.

Most of the hooks are present in imx-media-fim.c to use the hardware 
input capture to measure frame intervals. The only missing piece is the 
ICAP driver itself.

The FIM is explained in the imx.rst doc. It is configured and enabled 
via v4l2 controls. It will report a v4l2_event to userspace during 
streaming when a bad frame interval is detected. The rolling video / 
vsync problem caused by the initial corrupt frames from the adv718x 
produces bad frame intervals, thus the FIM event can be used to restart 
streaming, which will correct vsync (as long as the bad initial frames 
are skipped, which is the other piece of this problem that needs an 
acceptable solution).

The FIM can still be used now, without input capture support, but it is 
marking frame intervals using the monotonic time in the dma completion 
irq handler, which is not always accurate due to irq latency, depending 
on system load. So this method can sometimes report bad frame interval 
events that aren't real.

Steve

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Steve,

On Tue, Oct 15, 2019 at 3:19 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> I submitted the ICAP driver patch quite a while ago, it was ~2 yrs ago I
> think. Can't seem to find the link unfortunately.
>
> I'll work on updating the driver and retesting, and try resubmitting again.
>
> Most of the hooks are present in imx-media-fim.c to use the hardware
> input capture to measure frame intervals. The only missing piece is the
> ICAP driver itself.
>
> The FIM is explained in the imx.rst doc. It is configured and enabled
> via v4l2 controls. It will report a v4l2_event to userspace during

Thanks for the explanation.

I read through imx.rst, but it was not clear for me how to actually
configure/enable it via v4l2 controls.

Could you please provide an example on how I can configure/enable FIM?

Also, would you have an example on how to achieve the following
behavior explained at imx.rst?

"If the FIM detects a bad frame interval, the ipuX_csiY subdev will
send the event
V4L2_EVENT_IMX_FRAME_INTERVAL_ERROR. Userland can register with
the FIM event notification on the ipuX_csiY subdev device node.
Userland can issue a streaming restart when this event is received
to correct the rolling/split image."

Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

On 10/15/19 12:11 PM, Fabio Estevam wrote:
> Hi Steve,
>
> On Tue, Oct 15, 2019 at 3:19 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>> I submitted the ICAP driver patch quite a while ago, it was ~2 yrs ago I
>> think. Can't seem to find the link unfortunately.
>>
>> I'll work on updating the driver and retesting, and try resubmitting again.
>>
>> Most of the hooks are present in imx-media-fim.c to use the hardware
>> input capture to measure frame intervals. The only missing piece is the
>> ICAP driver itself.
>>
>> The FIM is explained in the imx.rst doc. It is configured and enabled
>> via v4l2 controls. It will report a v4l2_event to userspace during
> Thanks for the explanation.
>
> I read through imx.rst, but it was not clear for me how to actually
> configure/enable it via v4l2 controls.
>
> Could you please provide an example on how I can configure/enable FIM?
>
> Also, would you have an example on how to achieve the following
> behavior explained at imx.rst?
>
> "If the FIM detects a bad frame interval, the ipuX_csiY subdev will
> send the event
> V4L2_EVENT_IMX_FRAME_INTERVAL_ERROR. Userland can register with
> the FIM event notification on the ipuX_csiY subdev device node.
> Userland can issue a streaming restart when this event is received
> to correct the rolling/split image."

Here's a quick example that uses the end-of-frame method to measure fi's 
(all other FIM controls are left at the default values):

v4l2-ctl -d0 --set-ctrl=fim_enable=1
# disable input capture method
v4l2-ctl -d0 --set-ctrl=fim_input_capture_edge=0
v4l2-ctl -d0 --stream-mmap

This example will enable the FIM in Input Capture mode (when it is 
actually supported), with no skipped frames at stream on, an interval 
error tolerance of 20 usec, and no averaging:

v4l2-ctl -d0 --set-ctrl=fim_enable=1
# enable input capture method, on rising edges
v4l2-ctl -d0 --set-ctrl=fim_input_capture_edge=1
v4l2-ctl -d0 --set-ctrl=fim_input_capture_channel=0
v4l2-ctl -d0 --set-ctrl=fim_tolerance_min=20
v4l2-ctl -d0 --set-ctrl=fim_num_skip=0
v4l2-ctl -d0 --set-ctrl=fim_num_average=1
v4l2-ctl -d0 --stream-mmap

While streaming, bad frame interval events can be polled with:

v4l2-ctl -d0 --wait-for-event=0x08000001

I plan to post an update to the imx.rst doc with these FIM usage examples.

I updated the i.MX GPT input capture driver patch and re-tested on my 
modified SabreAuto (CSI0_VSYNC signal routed to SD1_DAT0 pad), and 
re-posted, you are cc'ed. Hopefully this will actually get looked at 
this time.

Steve

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Steve,

On Tue, Oct 15, 2019 at 10:18 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> Here's a quick example that uses the end-of-frame method to measure fi's
> (all other FIM controls are left at the default values):

Excellent! That was what I was looking for. Will test it soon.

> v4l2-ctl -d0 --wait-for-event=0x08000001
>
> I plan to post an update to the imx.rst doc with these FIM usage examples.

Great, that will be helpful!

> I updated the i.MX GPT input capture driver patch and re-tested on my
> modified SabreAuto (CSI0_VSYNC signal routed to SD1_DAT0 pad), and
> re-posted, you are cc'ed. Hopefully this will actually get looked at
> this time.

Will help reviewing/testing it.

Thanks!

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Steve,

On Tue, Oct 15, 2019 at 10:18 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> Here's a quick example that uses the end-of-frame method to measure fi's
> (all other FIM controls are left at the default values):
>
> v4l2-ctl -d0 --set-ctrl=fim_enable=1
> # disable input capture method
> v4l2-ctl -d0 --set-ctrl=fim_input_capture_edge=0
> v4l2-ctl -d0 --stream-mmap

What needs to be done for 'fim_enable' to be available?

I am using a mx6dl-sabreauto, and this is what I get:

# v4l2-ctl -d2 --set-ctrl=fim_enable=1
unknown control 'fim_enable'

Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

On 10/16/19 6:04 AM, Fabio Estevam wrote:
> Hi Steve,
>
> On Tue, Oct 15, 2019 at 10:18 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>> Here's a quick example that uses the end-of-frame method to measure fi's
>> (all other FIM controls are left at the default values):
>>
>> v4l2-ctl -d0 --set-ctrl=fim_enable=1
>> # disable input capture method
>> v4l2-ctl -d0 --set-ctrl=fim_input_capture_edge=0
>> v4l2-ctl -d0 --stream-mmap
> What needs to be done for 'fim_enable' to be available?
>
> I am using a mx6dl-sabreauto, and this is what I get:
>
> # v4l2-ctl -d2 --set-ctrl=fim_enable=1
> unknown control 'fim_enable'

If /dev/video2 is the "ipu1_ic_prpvf capture" node, it's because FIM is 
not yet available on those nodes. The FIM is only available on the 
"ipuX_csiY capture" nodes. It's on my plate to fix that.

But also remember that the FIM controls are inherited from the ipuX_csiY 
subdevices when the link from ipuX_csiY is enabled, so while the 
controls are always available on ipuX_csiY, they become available on 
"ipuX_csiY capture" nodes when the link is enabled.

Steve

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

On Wed, Oct 16, 2019 at 2:31 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> If /dev/video2 is the "ipu1_ic_prpvf capture" node, it's because FIM is
> not yet available on those nodes. The FIM is only available on the
> "ipuX_csiY capture" nodes. It's on my plate to fix that.

On a 5.3.6 kernel on imx6dl-sabreauto:

# v4l2-ctl --list-devices
imx-media (platform:imx-media):
        /dev/media0

imx-media-capture (platform:ipu1_csi0):
        /dev/video0

imx-media-capture (platform:ipu1_csi1):
        /dev/video3

imx-media-capture (platform:ipu1_ic_prpenc):
        /dev/video1

imx-media-capture (platform:ipu1_ic_prpvf):
        /dev/video2


# v4l2-ctl -d0  --list-ctrls
# v4l2-ctl -d1  --list-ctrls
# v4l2-ctl -d2  --list-ctrls

User Controls

                     brightness 0x00980900 (int)    : min=-128 max=127 step=1 dr
                       contrast 0x00980901 (int)    : min=0 max=255 step=1 defar
                     saturation 0x00980902 (int)    : min=0 max=255 step=1 defar
                            hue 0x00980903 (int)    : min=-127 max=128 step=1 dr
                horizontal_flip 0x00980914 (bool)   : default=0 value=0
                  vertical_flip 0x00980915 (bool)   : default=0 value=0
                         rotate 0x00980922 (int)    : min=0 max=270 step=90 deft
                 fast_switching 0x00981970 (bool)   : default=0 value=0

Image Processing Controls

             deinterlacing_mode 0x009f0904 (menu)   : min=0 max=3 default=3 val3
# v4l2-ctl -d3  --list-ctrls
#

I cannot see the FIM controls appearing in any device here.

Anything I am missing?

Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Steve Longerbeam]

On 10/16/19 11:54 AM, Fabio Estevam wrote:
> On Wed, Oct 16, 2019 at 2:31 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>> If /dev/video2 is the "ipu1_ic_prpvf capture" node, it's because FIM is
>> not yet available on those nodes. The FIM is only available on the
>> "ipuX_csiY capture" nodes. It's on my plate to fix that.
> On a 5.3.6 kernel on imx6dl-sabreauto:
>
> # v4l2-ctl --list-devices
> imx-media (platform:imx-media):
>          /dev/media0
>
> imx-media-capture (platform:ipu1_csi0):
>          /dev/video0
>
> imx-media-capture (platform:ipu1_csi1):
>          /dev/video3

FIM is available on the above nodes (/dev/video0 and /dev/video3), after 
enabling links to them. So please try:

# media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
# v4l2-ctl -d0  --list-ctrls

# media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
# v4l2-ctl -d3  --list-ctrls

Steve

>
> imx-media-capture (platform:ipu1_ic_prpenc):
>          /dev/video1
>
> imx-media-capture (platform:ipu1_ic_prpvf):
>          /dev/video2
>
>
> # v4l2-ctl -d0  --list-ctrls
> # v4l2-ctl -d1  --list-ctrls
> # v4l2-ctl -d2  --list-ctrls
>
> User Controls
>
>                       brightness 0x00980900 (int)    : min=-128 max=127 step=1 dr
>                         contrast 0x00980901 (int)    : min=0 max=255 step=1 defar
>                       saturation 0x00980902 (int)    : min=0 max=255 step=1 defar
>                              hue 0x00980903 (int)    : min=-127 max=128 step=1 dr
>                  horizontal_flip 0x00980914 (bool)   : default=0 value=0
>                    vertical_flip 0x00980915 (bool)   : default=0 value=0
>                           rotate 0x00980922 (int)    : min=0 max=270 step=90 deft
>                   fast_switching 0x00981970 (bool)   : default=0 value=0
>
> Image Processing Controls
>
>               deinterlacing_mode 0x009f0904 (menu)   : min=0 max=3 default=3 val3
> # v4l2-ctl -d3  --list-ctrls
> #
>
> I cannot see the FIM controls appearing in any device here.
>
> Anything I am missing?
>
> Thanks

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Steve,

On Wed, Oct 16, 2019 at 4:11 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> FIM is available on the above nodes (/dev/video0 and /dev/video3), after
> enabling links to them. So please try:
>
> # media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
> # v4l2-ctl -d0  --list-ctrls
>
> # media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
> # v4l2-ctl -d3  --list-ctrls

Thanks. This makes the FIM controls to appear.

However, if I run the following configuration so that I can get
Gstreamer pipeline to run (gst-launch-1.0 v4l2src device=/dev/video2 !
kmssink) then FIM does not appear.

# media-ctl -l "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]";
media-ctl -l "'ipu1_csi0':1 -> 'ipu1_vdic':0[1]"
media-ctl -l "'ipu1_vdic':2 -> 'ipu1_ic_prp':0[1]"
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]"

But I think this behavior is expected according to your previous explanation:

"If /dev/video2 is the "ipu1_ic_prpvf capture" node, it's because FIM is
not yet available on those nodes. The FIM is only available on the
"ipuX_csiY capture" nodes. It's on my plate to fix that."

Thanks for your patience in clarifying this.

Cheers

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Hans Verkuil]

On 16/10/2019 21:51, Fabio Estevam wrote:
> Hi Steve,
> 
> On Wed, Oct 16, 2019 at 4:11 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
> 
>> FIM is available on the above nodes (/dev/video0 and /dev/video3), after
>> enabling links to them. So please try:
>>
>> # media-ctl -l "'ipu1_csi0':2 -> 'ipu1_csi0 capture':0[1]"
>> # v4l2-ctl -d0  --list-ctrls
>>
>> # media-ctl -l "'ipu1_csi1':2 -> 'ipu1_csi1 capture':0[1]"
>> # v4l2-ctl -d3  --list-ctrls
> 
> Thanks. This makes the FIM controls to appear.
> 
> However, if I run the following configuration so that I can get
> Gstreamer pipeline to run (gst-launch-1.0 v4l2src device=/dev/video2 !
> kmssink) then FIM does not appear.
> 
> # media-ctl -l "'adv7180 4-0021':0 -> 'ipu1_csi0_mux':4[1]"
> media-ctl -l "'ipu1_csi0_mux':5 -> 'ipu1_csi0':0[1]";
> media-ctl -l "'ipu1_csi0':1 -> 'ipu1_vdic':0[1]"
> media-ctl -l "'ipu1_vdic':2 -> 'ipu1_ic_prp':0[1]"
> 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]"
> 
> But I think this behavior is expected according to your previous explanation:
> 
> "If /dev/video2 is the "ipu1_ic_prpvf capture" node, it's because FIM is
> not yet available on those nodes. The FIM is only available on the
> "ipuX_csiY capture" nodes. It's on my plate to fix that."
> 
> Thanks for your patience in clarifying this.
> 
> Cheers
> 

This discussion fizzled out, and in any case this patch no longer applies.

I'm marking it as 'Changes Requested' in patchwork.

Regards,

	Hans

Re: [PATCH] media: imx.rst: Update the imx6-sabreauto ADV7180 instructions

[Fabio Estevam]

Hi Hans,

On Thu, Jun 25, 2020 at 7:53 AM Hans Verkuil <hverkuil-cisco@xs4all.nl> wrote:

> This discussion fizzled out, and in any case this patch no longer applies.

The current document has the correct instructions for both the imx6qdl
and imx6q sabreauto boards, so this patch is no longer needed.

Thanks