adv7280: Scrolling images on imx6

adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi Tim,

I am back at trying to get a proper image captured by the ADV7280 on
an imx6dl board connected to the parallel CSI bus.

I tried your patches against 5.4 and 5.12:

https://github.com/Gateworks/linux-imx6/commit/959fbd4
and
https://patchwork.kernel.org/project/linux-media/patch/20190827215539.1286-1-mmichilot@gateworks.com/

,but I am still getting horizontal scrolling images after running:

media-ctl -l "'adv7180 0-0020':0 -> 'ipu1_csi1_mux':4[1]"
media-ctl -l "'ipu1_csi1_mux':5 -> 'ipu1_csi1':0[1]"
media-ctl -l "'ipu1_csi1':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]"
media-ctl -V "'adv7180 0-0020':0 [fmt:UYVY2X8/640x480 field:seq-tb]"
media-ctl -V "'ipu1_csi1_mux':5 [fmt:UYVY2X8/640x480]"
media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/640x480]"
media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/640x480 field:none]"
media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/640x480 field:none]"
media-ctl -V "'ipu1_ic_prpvf':1 [fmt:AYUV32/640x480 field:none]"
v4l2-ctl -d2 --set-fmt-video=field=none

gst-launch-1.0 v4l2src device=/dev/video2 ! kmssink

Also, I saw your commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.12&id=9483a3f8e1b58ba1d7cd21687d8d0a63a015c36b

,but adv7180_get_skip_frames() does not get called. Is there any
missing patch from the mix csi driver that should call
g_skip_frames()?

Any suggestions?

Thanks,

Fabio Estevam

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

On Mon, Apr 26, 2021 at 1:14 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> I am back at trying to get a proper image captured by the ADV7280 on
> an imx6dl board connected to the parallel CSI bus.
>
> I tried your patches against 5.4 and 5.12:
>
> https://github.com/Gateworks/linux-imx6/commit/959fbd4
> and
> https://patchwork.kernel.org/project/linux-media/patch/20190827215539.1286-1-mmichilot@gateworks.com/
>
> ,but I am still getting horizontal scrolling images after running:

The analog camera I am using produces PAL content. If I force the
initial curr_norm to PAL like this:

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index e780969cc2f2..4ffe85ef02a0 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -1346,7 +1346,7 @@ static int adv7180_probe(struct i2c_client *client,

  state->irq = client->irq;
  mutex_init(&state->mutex);
- state->curr_norm = V4L2_STD_NTSC;
+ state->curr_norm = V4L2_STD_PAL;
  if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
  state->powered = true;
  else

Then I get stable image and no more scrolling.

This patch from Steve is still needed though:
https://github.com/Gateworks/linux-imx6/commit/959fbd4

What would be the correct way to fix this NTSC versus PAL selection?

I tried passing the norm property like this, but it gives me an error:

# gst-launch-1.0 -v  v4l2src norm=PAL device=/dev/video2 ! kmssink
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 800
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 480
WARNING: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
Failed to set norm for device '/dev/video2'.
Additional debug info:
../gst-plugins-good-1.18.2/sys/v4l2/v4l2_calls.c(779):
gst_v4l2_set_norm (): /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
system error: Inappropriate ioctl for device

Appreciate any suggestions, thanks

Re: adv7280: Scrolling images on imx6

[Frieder Schrempf]

On 27.04.21 15:27, Fabio Estevam wrote:
> On Mon, Apr 26, 2021 at 1:14 PM Fabio Estevam <festevam@gmail.com> wrote:
>>
>> Hi Tim,
>>
>> I am back at trying to get a proper image captured by the ADV7280 on
>> an imx6dl board connected to the parallel CSI bus.
>>
>> I tried your patches against 5.4 and 5.12:
>>
>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGateworks%2Flinux-imx6%2Fcommit%2F959fbd4&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cbe8362b204924f7c141008d909804d4a%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637551268920096235%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2ZBhpF3FRT1hpXKH95KHnoNJm42p9gHzi6h54Fokvvk%3D&amp;reserved=0
>> and
>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fproject%2Flinux-media%2Fpatch%2F20190827215539.1286-1-mmichilot%40gateworks.com%2F&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cbe8362b204924f7c141008d909804d4a%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637551268920096235%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6h1N8qQ7J4w2xJujKOQp4spJDiOGLm%2FxTx0SIbBpgGs%3D&amp;reserved=0
>>
>> ,but I am still getting horizontal scrolling images after running:
> 
> The analog camera I am using produces PAL content. If I force the
> initial curr_norm to PAL like this:
> 
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index e780969cc2f2..4ffe85ef02a0 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -1346,7 +1346,7 @@ static int adv7180_probe(struct i2c_client *client,
> 
>    state->irq = client->irq;
>    mutex_init(&state->mutex);
> - state->curr_norm = V4L2_STD_NTSC;
> + state->curr_norm = V4L2_STD_PAL;
>    if (state->chip_info->flags & ADV7180_FLAG_RESET_POWERED)
>    state->powered = true;
>    else
> 
> Then I get stable image and no more scrolling.
> 
> This patch from Steve is still needed though:
> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FGateworks%2Flinux-imx6%2Fcommit%2F959fbd4&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Cbe8362b204924f7c141008d909804d4a%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637551268920096235%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2ZBhpF3FRT1hpXKH95KHnoNJm42p9gHzi6h54Fokvvk%3D&amp;reserved=0
> 
> What would be the correct way to fix this NTSC versus PAL selection?
> 
> I tried passing the norm property like this, but it gives me an error:
> 
> # gst-launch-1.0 -v  v4l2src norm=PAL device=/dev/video2 ! kmssink
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> /GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 800
> /GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 480
> WARNING: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> Failed to set norm for device '/dev/video2'.
> Additional debug info:
> ../gst-plugins-good-1.18.2/sys/v4l2/v4l2_calls.c(779):
> gst_v4l2_set_norm (): /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> system error: Inappropriate ioctl for device
> 
> Appreciate any suggestions, thanks

I don't know much about the v4l/media subsystem, but in my case I was 
able to switch to PAL like this:

v4l2-ctl --device /dev/v4l-subdev2 --set-standard PAL

Though your subdev is probably a different one as I was running this on 
i.MX8MM without IPU.

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi Frieder,

On Tue, Apr 27, 2021 at 10:33 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:

> I don't know much about the v4l/media subsystem, but in my case I was
> able to switch to PAL like this:
>
> v4l2-ctl --device /dev/v4l-subdev2 --set-standard PAL
>
> Though your subdev is probably a different one as I was running this on
> i.MX8MM without IPU.

Thanks, this does the trick!

I will try to upstream Steve's patch then.

Thanks a lot

Re: adv7280: Scrolling images on imx6

[Frieder Schrempf]

On 27.04.21 15:47, Fabio Estevam wrote:
> Hi Frieder,
> 
> On Tue, Apr 27, 2021 at 10:33 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
> 
>> I don't know much about the v4l/media subsystem, but in my case I was
>> able to switch to PAL like this:
>>
>> v4l2-ctl --device /dev/v4l-subdev2 --set-standard PAL
>>
>> Though your subdev is probably a different one as I was running this on
>> i.MX8MM without IPU.
> 
> Thanks, this does the trick!
> 
> I will try to upstream Steve's patch then.
> 
> Thanks a lot
> 

You're welcome!

Just a question: What issue do you see without Steve's patch?

I'm asking because with my setup with the imx7 CSI driver I'm getting a 
stable image, but it has a vertical and horizontal offset that seems to 
vary each time I enable the camera. I was wondering if this is maybe 
caused by the missing frame skipping.

But still I'm not really sure if we can compare this as I needed another 
patch for the adv7180 driver to get it working with the i.MX8MM in the 
first place: 
https://git.kontron-electronics.de/linux/linux/-/commit/0d90331a44d0f718b7327a94fc72612ddcb4ac0f.

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi Frieder,

On Tue, Apr 27, 2021 at 11:15 AM Frieder Schrempf
<frieder.schrempf@kontron.de> wrote:

> You're welcome!
>
> Just a question: What issue do you see without Steve's patch?

Without Steve's patch, I see the captured image slowly scrolling
vertically, like an old tube TV.

Once I apply it, the image becomes stable. The board I am currently
using is based on an imx6dl and the ADV7180 is connected to the
parallel CSI bus.

> I'm asking because with my setup with the imx7 CSI driver I'm getting a
> stable image, but it has a vertical and horizontal offset that seems to
> vary each time I enable the camera. I was wondering if this is maybe
> caused by the missing frame skipping.

Yes, maybe the imx7 CSI driver needs this initial frame skipping mechanism too.

> But still I'm not really sure if we can compare this as I needed another
> patch for the adv7180 driver to get it working with the i.MX8MM in the
> first place:
> https://git.kontron-electronics.de/linux/linux/-/commit/0d90331a44d0f718b7327a94fc72612ddcb4ac0f.

Thanks for sharing your git tree.

In the next days, I will work on a different imx6dl board, with the
adv7180 connected to the MIPI CSI2.

I see your tree contains some adv7180-m related patches. I will give
it a try and report.

Thanks,

Fabio Estevam

Re: adv7280: Scrolling images on imx6

[Frieder Schrempf]

On 27.04.21 16:28, Fabio Estevam wrote:
> Hi Frieder,
> 
> On Tue, Apr 27, 2021 at 11:15 AM Frieder Schrempf
> <frieder.schrempf@kontron.de> wrote:
> 
>> You're welcome!
>>
>> Just a question: What issue do you see without Steve's patch?
> 
> Without Steve's patch, I see the captured image slowly scrolling
> vertically, like an old tube TV.
> 
> Once I apply it, the image becomes stable. The board I am currently
> using is based on an imx6dl and the ADV7180 is connected to the
> parallel CSI bus.
> 
>> I'm asking because with my setup with the imx7 CSI driver I'm getting a
>> stable image, but it has a vertical and horizontal offset that seems to
>> vary each time I enable the camera. I was wondering if this is maybe
>> caused by the missing frame skipping.
> 
> Yes, maybe the imx7 CSI driver needs this initial frame skipping mechanism too.
> 
>> But still I'm not really sure if we can compare this as I needed another
>> patch for the adv7180 driver to get it working with the i.MX8MM in the
>> first place:
>> https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kontron-electronics.de%2Flinux%2Flinux%2F-%2Fcommit%2F0d90331a44d0f718b7327a94fc72612ddcb4ac0f&amp;data=04%7C01%7Cfrieder.schrempf%40kontron.de%7Ce015a3d00b6f4c26745b08d90988c0e2%7C8c9d3c973fd941c8a2b1646f3942daf1%7C0%7C0%7C637551305224798164%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=c2W1OlaTvbjBDxkbVvJJtgPCRnsi7sCIA1QgiF9nZEM%3D&amp;reserved=0.
> 
> Thanks for sharing your git tree.
> 
> In the next days, I will work on a different imx6dl board, with the
> adv7180 connected to the MIPI CSI2.
> 
> I see your tree contains some adv7180-m related patches. I will give
> it a try and report.
> 

Ah, I forgot that your setup uses the CSI interface and not MIPI CSI2. 
So it's indeed somewhat difficult to compare on that level.

Re: adv7280: Scrolling images on imx6

[Tim Harvey]

On Mon, Apr 26, 2021 at 9:14 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> I am back at trying to get a proper image captured by the ADV7280 on
> an imx6dl board connected to the parallel CSI bus.
>
> I tried your patches against 5.4 and 5.12:
>
> https://github.com/Gateworks/linux-imx6/commit/959fbd4
> and
> https://patchwork.kernel.org/project/linux-media/patch/20190827215539.1286-1-mmichilot@gateworks.com/
>
> ,but I am still getting horizontal scrolling images after running:
>
> media-ctl -l "'adv7180 0-0020':0 -> 'ipu1_csi1_mux':4[1]"
> media-ctl -l "'ipu1_csi1_mux':5 -> 'ipu1_csi1':0[1]"
> media-ctl -l "'ipu1_csi1':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]"
> media-ctl -V "'adv7180 0-0020':0 [fmt:UYVY2X8/640x480 field:seq-tb]"
> media-ctl -V "'ipu1_csi1_mux':5 [fmt:UYVY2X8/640x480]"
> media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/640x480]"
> media-ctl -V "'ipu1_vdic':2 [fmt:AYUV32/640x480 field:none]"
> media-ctl -V "'ipu1_ic_prp':2 [fmt:AYUV32/640x480 field:none]"
> media-ctl -V "'ipu1_ic_prpvf':1 [fmt:AYUV32/640x480 field:none]"
> v4l2-ctl -d2 --set-fmt-video=field=none
>
> gst-launch-1.0 v4l2src device=/dev/video2 ! kmssink
>
> Also, I saw your commit:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.12&id=9483a3f8e1b58ba1d7cd21687d8d0a63a015c36b
>
> ,but adv7180_get_skip_frames() does not get called. Is there any
> missing patch from the mix csi driver that should call
> g_skip_frames()?
>

Fabio,

GStreamer does not use the g_skip_frames feature as far as I know. If
it did then we likely wouldn't need Steves patch
https://github.com/Gateworks/linux-imx6/commit/959fbd4. His commit
explains the issue pretty well and yes, this should be mainlined in my
opinion unless someone presents a more proper way to handle it.

The proper way to set the video standard (PAL vs NTSC) is 'v4l2-ctl
--device /dev/v4l-subdev2 --set-standard PAL' (on the proper subdev of
course).

The https://patchwork.kernel.org/project/linux-media/patch/20190827215539.1286-1-mmichilot@gateworks.com/
patch is only needed if you are using adv7280 which I thought you
mentioned you were at some point. I still don't have a solution for
that other than add a kernel module parameter or something.

Best regards,

Tim

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi Tim,

On Tue, Apr 27, 2021 at 12:20 PM Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> GStreamer does not use the g_skip_frames feature as far as I know. If
> it did then we likely wouldn't need Steves patch
> https://github.com/Gateworks/linux-imx6/commit/959fbd4. His commit
> explains the issue pretty well and yes, this should be mainlined in my
> opinion unless someone presents a more proper way to handle it.

Yes, I have submitted Steve's patch to the list.

> The proper way to set the video standard (PAL vs NTSC) is 'v4l2-ctl
> --device /dev/v4l-subdev2 --set-standard PAL' (on the proper subdev of
> course).

Thanks, Frieder gave me the same suggestion and it worked.

> The https://patchwork.kernel.org/project/linux-media/patch/20190827215539.1286-1-mmichilot@gateworks.com/
> patch is only needed if you are using adv7280 which I thought you
> mentioned you were at some point. I still don't have a solution for
> that other than add a kernel module parameter or something.

Ops, sorry. The chip I am using is in fact an ADV7280 connected to the
CSI parallel bus.

I didn't apply Matthew's patch and I am getting a good quality image though.

What is the problem you notice when not applying this patch?

Thanks

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi Tim,

On Tue, Apr 27, 2021 at 12:51 PM Fabio Estevam <festevam@gmail.com> wrote:

> I didn't apply Matthew's patch and I am getting a good quality image though.
>
> What is the problem you notice when not applying this patch?

Now I see the problem when an NTSC camera is used.

I confirm that Matthew's patch fixes the issue.

Thanks

Re: adv7280: Scrolling images on imx6

[Tim Harvey]

On Thu, May 20, 2021 at 4:42 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Tim,
>
> On Tue, Apr 27, 2021 at 12:51 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> > I didn't apply Matthew's patch and I am getting a good quality image though.
> >
> > What is the problem you notice when not applying this patch?
>
> Now I see the problem when an NTSC camera is used.
>
> I confirm that Matthew's patch fixes the issue.
>

Fabio,

Yes sorry for not getting back to you on this. I actually haven't had
time to go find the proper board and test it. You are correct, the
scrolling sync issue on the adv7280 occurs with NTSC. I'm still not
sure how best to fix this as the patch was Nak'd. As discussed I guess
we will have to do something with a kernel param. I am told that we
should not attempt to normalize default configurations among the
various chips the adv7180 driver supports as it may break someone's
current use of them.

Tim

Re: adv7280: Scrolling images on imx6

[Fabio Estevam]

Hi  Tim,

On Fri, May 21, 2021 at 11:25 AM Tim Harvey <tharvey@gateworks.com> wrote:

> Fabio,
>
> Yes sorry for not getting back to you on this. I actually haven't had
> time to go find the proper board and test it. You are correct, the
> scrolling sync issue on the adv7280 occurs with NTSC. I'm still not
> sure how best to fix this as the patch was Nak'd. As discussed I guess
> we will have to do something with a kernel param. I am told that we
> should not attempt to normalize default configurations among the
> various chips the adv7180 driver supports as it may break someone's
> current use of them.

Yes, I am interested in getting this fix upstream.

I will try to rework Matthew's patch by adding a device tree property
so that current systems are not affected by this change and will post
a patch for discussion soon.

BTW, I have sent a v3 of Steve's fix for skipping initial bad frames:
https://www.spinics.net/lists/linux-media/msg192778.html

It would be nice if you could give your Reviewed-by or Tested-by.

Thanks