imx6ull capture from OV5640

  Hi,

I am trying to capture from a parallel OV5640 on a imx6ull-evk board.

Here are the device tree changes:
https://pastebin.com/raw/PZpJjagJ

First, I got the following warning:

[    7.788866] csi: Registered csi capture as /dev/video1
[    7.797382] ------------[ cut here ]------------
[    7.802141] WARNING: CPU: 0 PID: 1 at
drivers/staging/media/imx/imx7-media-csi.c:1168
imx7_csi_notify_bound+0x40/0x50
[    7.813116] Modules linked in:
[    7.816436] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.10.0-next-20201223-00003-gaaee78ed150-dirty #304
[    7.826015] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[    7.832281] [<c0111a68>] (unwind_backtrace) from [<c010c068>]
(show_stack+0x10/0x14)
[    7.840151] [<c010c068>] (show_stack) from [<c0e14570>]
(dump_stack+0xe0/0x10c)
[    7.847570] [<c0e14570>] (dump_stack) from [<c0125a7c>] (__warn+0x104/0x118)
[    7.854734] [<c0125a7c>] (__warn) from [<c0125b38>]
(warn_slowpath_fmt+0xa8/0xb8)
[    7.862326] [<c0125b38>] (warn_slowpath_fmt) from [<c0a66e0c>]
(imx7_csi_notify_bound+0x40/0x50)
[    7.871227] [<c0a66e0c>] (imx7_csi_notify_bound) from [<c09ae084>]
(v4l2_async_match_notify+0x50/0x124)

To avoid the warning I did:

 }

# media-ctl -p
Media controller API version 5.10.0

Media device information
------------------------
driver          imx7-csi
model           imx-media
serial
bus info
hw revision     0x0
driver version  5.10.0

Device topology
- entity 1: csi (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
        pad0: Sink
                [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:lim-range]
                <- "ov5640 1-003c":0 []
        pad1: Source
                [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:lim-range]
                -> "csi capture":0 []

- entity 4: csi capture (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video1
        pad0: Sink
                <- "csi":1 []

- entity 10: ov5640 1-003c (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev1
        pad0: Source
                [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:full-range]
                -> "csi":0 []
And then:

media-ctl -l "'ov5640 1-003c':0 -> 'csi':0[1]"
media-ctl -l "'csi':1 -> 'csi capture':0[1]"
media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
media-ctl -V "'csi':0 [fmt:AYUV32/640x480]"

When trying to capture via v42-ctl:
# v4l2-ctl --stream-mmap -d /dev/video1
[  411.627032] csi: capture format not valid

Or with gstreamer:

# gst-launch-1.0 v4l2src device=/dev/video1 ! fakesink
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
[  439.933324] csi: pipeline start failed with -19

Any suggestions?

Thanks,

Fabio Estevam
  

Hi Fabio,

On Wed, Dec 30, 2020 at 02:45:41PM -0300, Fabio Estevam wrote:
> Hi,
> 
> I am trying to capture from a parallel OV5640 on a imx6ull-evk board.
> 
> Here are the device tree changes:
> https://pastebin.com/raw/PZpJjagJ
> 
> First, I got the following warning:
> 
> [    7.788866] csi: Registered csi capture as /dev/video1
> [    7.797382] ------------[ cut here ]------------
> [    7.802141] WARNING: CPU: 0 PID: 1 at
> drivers/staging/media/imx/imx7-media-csi.c:1168
> imx7_csi_notify_bound+0x40/0x50
> [    7.813116] Modules linked in:
> [    7.816436] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
> 5.10.0-next-20201223-00003-gaaee78ed150-dirty #304
> [    7.826015] Hardware name: Freescale i.MX6 Ultralite (Device Tree)
> [    7.832281] [<c0111a68>] (unwind_backtrace) from [<c010c068>]
> (show_stack+0x10/0x14)
> [    7.840151] [<c010c068>] (show_stack) from [<c0e14570>]
> (dump_stack+0xe0/0x10c)
> [    7.847570] [<c0e14570>] (dump_stack) from [<c0125a7c>] (__warn+0x104/0x118)
> [    7.854734] [<c0125a7c>] (__warn) from [<c0125b38>]
> (warn_slowpath_fmt+0xa8/0xb8)
> [    7.862326] [<c0125b38>] (warn_slowpath_fmt) from [<c0a66e0c>]
> (imx7_csi_notify_bound+0x40/0x50)
> [    7.871227] [<c0a66e0c>] (imx7_csi_notify_bound) from [<c09ae084>]
> (v4l2_async_match_notify+0x50/0x124)
> 
> To avoid the warning I did:
> 
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1164,12 +1164,14 @@ static int imx7_csi_notify_bound(struct
> v4l2_async_notifier *notifier,
>         struct imx7_csi *csi = imx7_csi_notifier_to_dev(notifier);
>         struct media_pad *sink = &csi->sd.entity.pads[IMX7_CSI_PAD_SINK];
> 
> -       /* The bound subdev must always be the CSI mux */
> -       if (WARN_ON(sd->entity.function != MEDIA_ENT_F_VID_MUX))
> -               return -ENXIO;
> +       if (csi->is_csi2) {
> +               /* The bound subdev must always be the CSI mux */
> +               if (WARN_ON(sd->entity.function != MEDIA_ENT_F_VID_MUX))
> +                       return -ENXIO;
> 
> -       /* Mark it as such via its group id */
> -       sd->grp_id = IMX_MEDIA_GRP_ID_CSI_MUX;
> +               /* Mark it as such via its group id */
> +               sd->grp_id = IMX_MEDIA_GRP_ID_CSI_MUX;
> +       }
> 
>         return v4l2_create_fwnode_links_to_pad(sd, sink);
>  }

That's not right, csi->is_csi2 is a flag that indicates if the current
input to the CSI comes from the CSI-2 receiver.

It looks like the i.MX6ULL is missing the MIPI CSI-2 receiver and thus
also the corresponding video mux. The WARN_ON() should thus indeed by
bypassed, but only for devices that don't have the video mux. I wouldn't
be surprised if other adaptations would be needed in the code.

On a side note, the driver is a bit hard to read, mixing i.MX6 and i.MX7
support leads to quite a bit of spaghetti code (and i.MX6 is a misnommer
to start with, as shown by the i.MX6ULL that has a CSI, not an IPUv3).
We should split the driver in two, rename i.MX7 support to CSI and i.MX6
to IPUv3, but that will be a large effort.

> # media-ctl -p
> Media controller API version 5.10.0
> 
> Media device information
> ------------------------
> driver          imx7-csi
> model           imx-media
> serial
> bus info
> hw revision     0x0
> driver version  5.10.0
> 
> Device topology
> - entity 1: csi (2 pads, 2 links)
>             type V4L2 subdev subtype Unknown flags 0
>             device node name /dev/v4l-subdev0
>         pad0: Sink
>                 [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb
> xfer:srgb ycbcr:601 quantization:lim-range]
>                 <- "ov5640 1-003c":0 []
>         pad1: Source
>                 [fmt:UYVY8_2X8/800x600 field:none colorspace:srgb
> xfer:srgb ycbcr:601 quantization:lim-range]
>                 -> "csi capture":0 []
> 
> - entity 4: csi capture (1 pad, 1 link)
>             type Node subtype V4L flags 0
>             device node name /dev/video1
>         pad0: Sink
>                 <- "csi":1 []
> 
> - entity 10: ov5640 1-003c (1 pad, 1 link)
>              type V4L2 subdev subtype Sensor flags 0
>              device node name /dev/v4l-subdev1
>         pad0: Source
>                 [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb
> xfer:srgb ycbcr:601 quantization:full-range]
>                 -> "csi":0 []
> And then:
> 
> media-ctl -l "'ov5640 1-003c':0 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
> media-ctl -V "'csi':0 [fmt:AYUV32/640x480]"
> 
> When trying to capture via v42-ctl:
> # v4l2-ctl --stream-mmap -d /dev/video1
> [  411.627032] csi: capture format not valid
> 
> Or with gstreamer:
> 
> # gst-launch-1.0 v4l2src device=/dev/video1 ! fakesink
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> Pipeline is PREROLLED ...
> Setting pipeline to PLAYING ...
> New clock: GstSystemClock
> [  439.933324] csi: pipeline start failed with -19
> 
> Any suggestions?
> 
> Thanks,
> 
> Fabio Estevam
  

Hi Laurent,

Thanks for your comments.

On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> That's not right, csi->is_csi2 is a flag that indicates if the current
> input to the CSI comes from the CSI-2 receiver.
>
> It looks like the i.MX6ULL is missing the MIPI CSI-2 receiver and thus
> also the corresponding video mux. The WARN_ON() should thus indeed by
> bypassed, but only for devices that don't have the video mux. I wouldn't

Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP block.

They only have a parallel CSI interface, and no video mux is present.

So the csi->is_csi2 check I did seems correct, right?

> be surprised if other adaptations would be needed in the code.

Yes, I found other paths that miss the csi->is_csi2 check too.

Thanks,

Fabio Estevam
  

Hi Fabio,

On Mon, Jan 04, 2021 at 08:34:48AM -0300, Fabio Estevam wrote:
> On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart wrote:
> 
> > That's not right, csi->is_csi2 is a flag that indicates if the current
> > input to the CSI comes from the CSI-2 receiver.
> >
> > It looks like the i.MX6ULL is missing the MIPI CSI-2 receiver and thus
> > also the corresponding video mux. The WARN_ON() should thus indeed by
> > bypassed, but only for devices that don't have the video mux. I wouldn't
> 
> Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP block.
> 
> They only have a parallel CSI interface, and no video mux is present.
> 
> So the csi->is_csi2 check I did seems correct, right?

I don't think so. csi->is_csi2 tells if the currently selected input of
the video mux is the CSI-2 receiver, not if there's a CSI-2 receiver
present in the device. csi->is_csi2 should of course always be false
when there's no CSI-2 receiver, but it can be false when a CSI-2
receiver is present and the currently selected input is the parallel
input.

> > be surprised if other adaptations would be needed in the code.
> 
> Yes, I found other paths that miss the csi->is_csi2 check too.
  

Hi,
catching up with this thread.

On Mon, Jan 04, 2021 at 03:08:58PM +0200, Laurent Pinchart wrote:
> Hi Fabio,
> 
> On Mon, Jan 04, 2021 at 08:34:48AM -0300, Fabio Estevam wrote:
> > On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart wrote:
> > 
> > > That's not right, csi->is_csi2 is a flag that indicates if the
> > > current input to the CSI comes from the CSI-2 receiver.
> > >
> > > It looks like the i.MX6ULL is missing the MIPI CSI-2 receiver
> > > and thus also the corresponding video mux. The WARN_ON() should
> > > thus indeed by bypassed, but only for devices that don't have
> > > the video mux. I wouldn't
> > 
> > Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP block.
> > 
> > They only have a parallel CSI interface, and no video mux is
> > present.
> > 
> > So the csi->is_csi2 check I did seems correct, right?
> 
> I don't think so. csi->is_csi2 tells if the currently selected input
> of the video mux is the CSI-2 receiver, not if there's a CSI-2
> receiver present in the device. csi->is_csi2 should of course always
> be false when there's no CSI-2 receiver, but it can be false when a
> CSI-2 receiver is present and the currently selected input is the
> parallel input.

Laurent is correct here. That flag indicates if CSI-2 is the selected
input for the video mux.

> 
> > > be surprised if other adaptations would be needed in the code.

I really only had the warp7 board which only had the csi2 as video mux
input, never got the chance to test it with a parallel input. And the
driver expects that we always have a mux. I was not even aware that an
imx6 would have the same csi ip.

but from the error outputs looks issues getting the format around the
imx7_csi_{try, get}_fmt.

------ 
Cheers, 
    Rui

> > 
> > Yes, I found other paths that miss the csi->is_csi2 check too.
> 
> -- Regards,
> 
> Laurent Pinchart
  

Hi Rui,

On Mon, Jan 04, 2021 at 01:45:11PM +0000, Rui Miguel Silva wrote:
> Hi,
> catching up with this thread.
> 
> On Mon, Jan 04, 2021 at 03:08:58PM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 04, 2021 at 08:34:48AM -0300, Fabio Estevam wrote:
> > > On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart wrote:
> > > 
> > > > That's not right, csi->is_csi2 is a flag that indicates if the
> > > > current input to the CSI comes from the CSI-2 receiver.
> > > >
> > > > It looks like the i.MX6ULL is missing the MIPI CSI-2 receiver
> > > > and thus also the corresponding video mux. The WARN_ON() should
> > > > thus indeed by bypassed, but only for devices that don't have
> > > > the video mux. I wouldn't
> > > 
> > > Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP block.
> > > 
> > > They only have a parallel CSI interface, and no video mux is
> > > present.
> > > 
> > > So the csi->is_csi2 check I did seems correct, right?
> > 
> > I don't think so. csi->is_csi2 tells if the currently selected input
> > of the video mux is the CSI-2 receiver, not if there's a CSI-2
> > receiver present in the device. csi->is_csi2 should of course always
> > be false when there's no CSI-2 receiver, but it can be false when a
> > CSI-2 receiver is present and the currently selected input is the
> > parallel input.
> 
> Laurent is correct here. That flag indicates if CSI-2 is the selected
> input for the video mux.
> 
> > > > be surprised if other adaptations would be needed in the code.
> 
> I really only had the warp7 board which only had the csi2 as video mux
> input, never got the chance to test it with a parallel input. And the
> driver expects that we always have a mux. I was not even aware that an
> imx6 would have the same csi ip.
> 
> but from the error outputs looks issues getting the format around the
> imx7_csi_{try, get}_fmt.

Do you still have the hardware, would you be able to test a patch series
?

> > > Yes, I found other paths that miss the csi->is_csi2 check too.
  

Hi Laurent,
On Mon, Jan 04, 2021 at 03:50:07PM +0200, Laurent Pinchart wrote:
> Hi Rui,
> 
> On Mon, Jan 04, 2021 at 01:45:11PM +0000, Rui Miguel Silva wrote:
> > Hi, catching up with this thread.
> > 
> > On Mon, Jan 04, 2021 at 03:08:58PM +0200, Laurent Pinchart wrote:
> > > On Mon, Jan 04, 2021 at 08:34:48AM -0300, Fabio Estevam wrote:
> > > > On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart wrote:
> > > > 
> > > > > That's not right, csi->is_csi2 is a flag that indicates if
> > > > > the current input to the CSI comes from the CSI-2 receiver.
> > > > >
> > > > > It looks like the i.MX6ULL is missing the MIPI CSI-2
> > > > > receiver and thus also the corresponding video mux. The
> > > > > WARN_ON() should thus indeed by bypassed, but only for
> > > > > devices that don't have the video mux. I wouldn't
> > > > 
> > > > Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP
> > > > block.
> > > > 
> > > > They only have a parallel CSI interface, and no video mux is
> > > > present.
> > > > 
> > > > So the csi->is_csi2 check I did seems correct, right?
> > > 
> > > I don't think so. csi->is_csi2 tells if the currently selected
> > > input of the video mux is the CSI-2 receiver, not if there's a
> > > CSI-2 receiver present in the device. csi->is_csi2 should of
> > > course always be false when there's no CSI-2 receiver, but it
> > > can be false when a CSI-2 receiver is present and the currently
> > > selected input is the parallel input.
> > 
> > Laurent is correct here. That flag indicates if CSI-2 is the
> > selected input for the video mux.
> > 
> > > > > be surprised if other adaptations would be needed in the
> > > > > code.
> > 
> > I really only had the warp7 board which only had the csi2 as video
> > mux input, never got the chance to test it with a parallel input.
> > And the driver expects that we always have a mux. I was not even
> > aware that an imx6 would have the same csi ip.
> > 
> > but from the error outputs looks issues getting the format around
> > the imx7_csi_{try, get}_fmt.
> 
> Do you still have the hardware, would you be able to test a patch
> series ?

Yeah, I have it somewhere... it could take a couple of days to
restore the setup, but possible for sure.

------
Cheers,
     Rui

> 
> > > > Yes, I found other paths that miss the csi->is_csi2 check too.
> 
> -- Regards,
> 
> Laurent Pinchart
  

H Rui,

On Mon, Jan 4, 2021 at 10:45 AM Rui Miguel Silva <rmfrfs@gmail.com> wrote:

> I really only had the warp7 board which only had the csi2 as video mux
> input, never got the chance to test it with a parallel input. And the
> driver expects that we always have a mux. I was not even aware that an
> imx6 would have the same csi ip.

Please check the following commit:

commit 0486a18ce82bd00d69ddc0fab8faa4b80df2117b
Author: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
Date:   Wed Jul 31 13:33:30 2019 -0300

    media: imx7-media-csi: add i.MX6UL support

    i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
    to imx7-media-csi driver.

    Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
    Reviewed-by: Rui Miguel Silva <rmfrfs@gmail.com>
    Reviewed-by: Fabio Estevam <festevam@gmail.com>
    Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
    Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
  

Hi Fabio,
On Mon, Jan 04, 2021 at 01:21:53PM -0300, Fabio Estevam wrote:
> H Rui,
> 
> On Mon, Jan 4, 2021 at 10:45 AM Rui Miguel Silva <rmfrfs@gmail.com>
> wrote:
> 
> > I really only had the warp7 board which only had the csi2 as video
> > mux input, never got the chance to test it with a parallel input.
> > And the driver expects that we always have a mux. I was not even
> > aware that an imx6 would have the same csi ip.
> 
> Please check the following commit:

yeah, of course I am aware of this patch, when I said I was not aware
any imx6 had the same csi ip was at the time of creating the initial
imx7 driver, not now.

Now I know it, but I never test in a parallel input scenario, the
changes from Sebastien in that patch were very small and specific to
setup csi for a parallel input and I think they worked fro him and did
not break the mipi-csi input type.

------
Cheers,
     Rui
> 
> commit 0486a18ce82bd00d69ddc0fab8faa4b80df2117b Author: Sébastien
> Szymanski <sebastien.szymanski@armadeus.com> Date:   Wed Jul 31
> 13:33:30 2019 -0300
> 
>     media: imx7-media-csi: add i.MX6UL support
> 
>     i.MX7 and i.MX6UL/L have the same CSI controller. So add
>     i.MX6UL/L support to imx7-media-csi driver.
> 
>     Signed-off-by: Sébastien Szymanski
>     <sebastien.szymanski@armadeus.com> Reviewed-by: Rui Miguel Silva
>     <rmfrfs@gmail.com> Reviewed-by: Fabio Estevam
>     <festevam@gmail.com> Signed-off-by: Sakari Ailus
>     <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho
>     Chehab <mchehab+samsung@kernel.org>
  

Hi Fabio,

On 12/30/20 6:45 PM, Fabio Estevam wrote:
> Hi,
> 
> I am trying to capture from a parallel OV5640 on a imx6ull-evk board.
> 
> Here are the device tree changes:
> https://pastebin.com/raw/PZpJjagJ

Don't you need

bus-type = <5>; // V4L2_FWNODE_BUS_TYPE_PARALLEL

in the parallel_from_ov5640 endpoint node ?

Regards,

> 
> Any suggestions?
> 
> Thanks,
> 
> Fabio Estevam
>
  

Hi Sébastien,

On Mon, Jan 4, 2021 at 2:19 PM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:

> Don't you need
>
> bus-type = <5>; // V4L2_FWNODE_BUS_TYPE_PARALLEL
>
> in the parallel_from_ov5640 endpoint node ?

Thanks for your suggestion, but I still get the warning and csi does not probe.

The modified dts is:
https://pastebin.com/raw/xENc5M5q

Could you please share your imx6ul board dts file that you used to
test camera capture?

Were you able to capture via Gstreamer? If so, please also share the
media-ctl and gst pipelines that you used.

Thanks,

Fabio Estevam
  

Hi Sébastien,

On Mon, Jan 4, 2021 at 8:15 PM Fabio Estevam <festevam@gmail.com> wrote:

> Could you please share your imx6ul board dts file that you used to
> test camera capture?

This dts allows csi to probe fine now:
https://pastebin.com/raw/7GK5dAWD

> Were you able to capture via Gstreamer? If so, please also share the
> media-ctl and gst pipelines that you used.

I am trying to capture via Gstreamer now. If you managed to get it
working, please share your media-ctl setup and Gst pipeline.

Thanks for your help!
  

Hi Fabio,

On 1/5/21 12:56 AM, Fabio Estevam wrote:
> Hi Sébastien,
> 
> On Mon, Jan 4, 2021 at 8:15 PM Fabio Estevam <festevam@gmail.com> wrote:
> 
>> Could you please share your imx6ul board dts file that you used to
>> test camera capture?
> 
> This dts allows csi to probe fine now:
> https://pastebin.com/raw/7GK5dAWD
> 
>> Were you able to capture via Gstreamer? If so, please also share the
>> media-ctl and gst pipelines that you used.
> 
> I am trying to capture via Gstreamer now. If you managed to get it
> working, please share your media-ctl setup and Gst pipeline.

I just tried it with kernel 5.4.84 with the following device tree (from
mainline kernel):
https://pastebin.com/w00EWZa5

I configured the pipelines with:
media-ctl -l "'ov5640 1-003c':0 -> 'csi':0[1]"
media-ctl -l "'csi':1 -> 'csi capture':0[1]"
media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_2X8/640x480 field:none]"

The topology is then:

Device topology
- entity 1: csi (2 pads, 2 links)
            type V4L2 subdev subtype Unknown flags 0
            device node name /dev/v4l-subdev0
        pad0: Sink
                [fmt:UYVY8_2X8/640x480 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:full-range]
                <- "ov5640 1-003c":0 [ENABLED]
        pad1: Source
                [fmt:UYVY8_2X8/640x480 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:full-range]
                -> "csi capture":0 [ENABLED]

- entity 4: csi capture (1 pad, 1 link)
            type Node subtype V4L flags 0
            device node name /dev/video1
        pad0: Sink
                <- "csi":1 [ENABLED]

- entity 10: ov5640 1-003c (1 pad, 1 link)
             type V4L2 subdev subtype Sensor flags 0
             device node name /dev/v4l-subdev1
        pad0: Source
                [fmt:UYVY8_2X8/640x480@1/30 field:none colorspace:srgb
xfer:srgb ycbcr:601 quantization:full-range]
                -> "csi":0 [ENABLED]

I used the following gstreamer pipeline to stream on the framebuffer:

gst-launch-1.0 v4l2src device=/dev/video1 ! v4l2convert ! fbdevsink

Regards,

> 
> Thanks for your help!
>
  

Hi Sébastien,

On Tue, Jan 5, 2021 at 6:49 AM Sébastien Szymanski
<sebastien.szymanski@armadeus.com> wrote:

> I used the following gstreamer pipeline to stream on the framebuffer:
>
> gst-launch-1.0 v4l2src device=/dev/video1 ! v4l2convert ! fbdevsink

Thanks, this helps and now the pipeline starts and I do see the camera
image on the LCD.

I switched to the same 5.4 you used just to make sure we are in the
same codebase.

I am getting the wrong colors though. The captured image is too pinky.

Do you get the correct colors on your test?

Also, I had to describe like the polarities like this:

hsync-active = <0>;
vsync-active = <1>;
pclk-sample = <0>;

because if I used the same polarities from your patch, then the
pipeline does not start.

Thanks for your help!
  

On Tue, Jan 5, 2021 at 10:19 AM Fabio Estevam <festevam@gmail.com> wrote:

> I switched to the same 5.4 you used just to make sure we are in the
> same codebase.

Just tested against next-20210105 and the original warning happens and
csi is no longer probed.

I am using the same dtb that worked on 5.4.84.

It looks like we have a regression.
  

On Tue, Jan 5, 2021 at 10:45 AM Fabio Estevam <festevam@gmail.com> wrote:

> Just tested against next-20210105 and the original warning happens and
> csi is no longer probed.
>
> I am using the same dtb that worked on 5.4.84.
>
> It looks like we have a regression.

And here is a fix that allows csi to probe:
https://pastebin.com/raw/g6ijDf2N

Makes sense?

There is another error though: I do not see the message below as seen
on 5.4 kernel:
[   10.690711] imx-media: ov5640 1-003c:0 -> csi:0

And the same pipeline that worked with 5.4 does not work with linux-next:

# media-ctl -l "'ov5640 1-003c':0 -> 'csi':0[1]"
media-ctl -l "'csi':1 -> 'csi capture':0[1]"
media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_2X8/320x240 field:none]"

# gst-launch-1.0 -v  v4l2src device=/dev/video1 ! v4l2convert ! fbdevsink
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstSystemClock
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
video/x-raw, format=(string)UYVY, width=(int)320, height=(int)240,
framerate=(fraction)30000/1001, interlace-mode=(string)progressive,
colorim
etry=(string)1:4:7:1
/GstPipeline:pipeline0/v4l2convert:v4l2convert0.GstPad:src: caps =
video/x-raw, format=(string)BGRx, width=(int)320, height=(int)240,
framerate=(fraction)30000/1001, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstFBDEVSink:fbdevsink0.GstPad:sink: caps =
video/[  421.495561] alloc_contig_range: [9c480, 9c4a6) PFNs busy
x-raw, format=(string)BGRx, width=(int)320, height=(int)240, fra[
421.504399] alloc_contig_range: [9c480, 9c4a6) PFNs busy
merate=(fraction)30000/1001, interlace-mode=(string)progressive
/GstPipeline:pipeline0/v4l2convert:v4l2convert0.GstPad:sink: c[
421.520989] alloc_contig_range: [9c480, 9c4a6) PFNs busy
aps = video/x-raw, format=(string)UYVY, width=(int)320, height=([
421.533523] csi: pipeline start failed with -19
int)240, framerate=(fraction)30000/1001,
interlace-mode=(string)progressive, colorimetry=(string)1:4:7:1
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
to allocate required memory.
Additional debug info:
../sys/v4l2/gstv4l2src.c(659): gst_v4l2src_decide_allocation ():
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
Buffer pool activation failed
Execution ended after 0:00:00.106613417
Setting pipeline to NULL ...
Freeing pipeline ...
#

Any ideas?

Thanks
  

Oi Fabio,
On Tue, Jan 05, 2021 at 10:45:59AM -0300, Fabio Estevam wrote:
> On Tue, Jan 5, 2021 at 10:19 AM Fabio Estevam <festevam@gmail.com>
> wrote:
> 
> > I switched to the same 5.4 you used just to make sure we are in
> > the same codebase.
> 
> Just tested against next-20210105 and the original warning happens
> and csi is no longer probed.
> 
> I am using the same dtb that worked on 5.4.84.
> 
> It looks like we have a regression.

Since we do not have many changes in there can you bisect?

maybe?
86e02d07871c2 media: imx5/6/7: csi: Mark a bound video mux as a CSI mux

------
Cheers,
     Rui
  

Hi Fabio,
On Tue, Jan 05, 2021 at 11:32:29AM -0300, Fabio Estevam wrote:
> On Tue, Jan 5, 2021 at 10:45 AM Fabio Estevam <festevam@gmail.com> wrote:
> 
> > Just tested against next-20210105 and the original warning happens and
> > csi is no longer probed.
> >
> > I am using the same dtb that worked on 5.4.84.
> >
> > It looks like we have a regression.
> 
> And here is a fix that allows csi to probe:
> https://pastebin.com/raw/g6ijDf2N
> 
> Makes sense?

yup.

> 
> There is another error though: I do not see the message below as seen
> on 5.4 kernel:
> [   10.690711] imx-media: ov5640 1-003c:0 -> csi:0
> 
> And the same pipeline that worked with 5.4 does not work with linux-next:
> 
> # media-ctl -l "'ov5640 1-003c':0 -> 'csi':0[1]"
> media-ctl -l "'csi':1 -> 'csi capture':0[1]"
> media-ctl -v -V "'ov5640 1-003c':0 [fmt:UYVY8_2X8/320x240 field:none]"
> 
> # gst-launch-1.0 -v  v4l2src device=/dev/video1 ! v4l2convert ! fbdevsink
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> Pipeline is PREROLLED ...
> Setting pipeline to PLAYING ...
> New clock: GstSystemClock
> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps =
> video/x-raw, format=(string)UYVY, width=(int)320, height=(int)240,
> framerate=(fraction)30000/1001, interlace-mode=(string)progressive,
> colorim
> etry=(string)1:4:7:1
> /GstPipeline:pipeline0/v4l2convert:v4l2convert0.GstPad:src: caps =
> video/x-raw, format=(string)BGRx, width=(int)320, height=(int)240,
> framerate=(fraction)30000/1001, interlace-mode=(string)progressive
> /GstPipeline:pipeline0/GstFBDEVSink:fbdevsink0.GstPad:sink: caps =
> video/[  421.495561] alloc_contig_range: [9c480, 9c4a6) PFNs busy
> x-raw, format=(string)BGRx, width=(int)320, height=(int)240, fra[
> 421.504399] alloc_contig_range: [9c480, 9c4a6) PFNs busy
> merate=(fraction)30000/1001, interlace-mode=(string)progressive
> /GstPipeline:pipeline0/v4l2convert:v4l2convert0.GstPad:sink: c[
> 421.520989] alloc_contig_range: [9c480, 9c4a6) PFNs busy
> aps = video/x-raw, format=(string)UYVY, width=(int)320, height=([
> 421.533523] csi: pipeline start failed with -19
> int)240, framerate=(fraction)30000/1001,
> interlace-mode=(string)progressive, colorimetry=(string)1:4:7:1
> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0: Failed
> to allocate required memory.
> Additional debug info:
> ../sys/v4l2/gstv4l2src.c(659): gst_v4l2src_decide_allocation ():
> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> Buffer pool activation failed
> Execution ended after 0:00:00.106613417
> Setting pipeline to NULL ...
> Freeing pipeline ...
> #
> 
> Any ideas?

can you see if the following patch make it work again?


8<----------------------------------------------------

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index a3f3df901704..fa8db9f1cfc8 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -499,6 +499,7 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
                                      struct v4l2_subdev_format *sink_fmt)
 {
        struct imx7_csi *csi = v4l2_get_subdevdata(sd);
+       struct media_entity *src;
        struct media_pad *pad;
        int ret;
 
@@ -509,11 +510,21 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev *sd,
        if (!csi->src_sd)
                return -EPIPE;
 
+       src = &csi->src_sd->entity;
+
+       /*
+        * if the source is neither a mux or csi2 get the one directly upstream
+        * from this csi
+        */
+       if (src->function != MEDIA_ENT_F_VID_IF_BRIDGE &&
+           src->function != MEDIA_ENT_F_VID_MUX)
+               src = &csi->sd.entity;
+
        /*
         * find the entity that is selected by the CSI mux. This is needed
         * to distinguish between a parallel or CSI-2 pipeline.
         */
-       pad = imx_media_pipeline_pad(&csi->src_sd->entity, 0, 0, true);
+       pad = imx_media_pipeline_pad(src, 0, 0, true);
        if (!pad)
                return -ENODEV;

> 
> Thanks
  

Hi Rui,

On Mon, Jan 04, 2021 at 02:05:11PM +0000, Rui Miguel Silva wrote:
> On Mon, Jan 04, 2021 at 03:50:07PM +0200, Laurent Pinchart wrote:
> > On Mon, Jan 04, 2021 at 01:45:11PM +0000, Rui Miguel Silva wrote:
> > > Hi, catching up with this thread.
> > > 
> > > On Mon, Jan 04, 2021 at 03:08:58PM +0200, Laurent Pinchart wrote:
> > > > On Mon, Jan 04, 2021 at 08:34:48AM -0300, Fabio Estevam wrote:
> > > > > On Mon, Jan 4, 2021 at 3:05 AM Laurent Pinchart wrote:
> > > > > 
> > > > > > That's not right, csi->is_csi2 is a flag that indicates if
> > > > > > the current input to the CSI comes from the CSI-2 receiver.
> > > > > >
> > > > > > It looks like the i.MX6ULL is missing the MIPI CSI-2
> > > > > > receiver and thus also the corresponding video mux. The
> > > > > > WARN_ON() should thus indeed by bypassed, but only for
> > > > > > devices that don't have the video mux. I wouldn't
> > > > > 
> > > > > Unlike i.MX7, i.MX6UL/i.MX6ULL do not have a MIPI CSI-2 IP
> > > > > block.
> > > > > 
> > > > > They only have a parallel CSI interface, and no video mux is
> > > > > present.
> > > > > 
> > > > > So the csi->is_csi2 check I did seems correct, right?
> > > > 
> > > > I don't think so. csi->is_csi2 tells if the currently selected
> > > > input of the video mux is the CSI-2 receiver, not if there's a
> > > > CSI-2 receiver present in the device. csi->is_csi2 should of
> > > > course always be false when there's no CSI-2 receiver, but it
> > > > can be false when a CSI-2 receiver is present and the currently
> > > > selected input is the parallel input.
> > > 
> > > Laurent is correct here. That flag indicates if CSI-2 is the
> > > selected input for the video mux.
> > > 
> > > > > > be surprised if other adaptations would be needed in the
> > > > > > code.
> > > 
> > > I really only had the warp7 board which only had the csi2 as video
> > > mux input, never got the chance to test it with a parallel input.
> > > And the driver expects that we always have a mux. I was not even
> > > aware that an imx6 would have the same csi ip.
> > > 
> > > but from the error outputs looks issues getting the format around
> > > the imx7_csi_{try, get}_fmt.
> > 
> > Do you still have the hardware, would you be able to test a patch
> > series ?
> 
> Yeah, I have it somewhere... it could take a couple of days to
> restore the setup, but possible for sure.

That would be really appreciated. I've CC'ed you on the patch series,
it's also available from

	git://linuxtv.org/pinchartl/media.git imx/csi/imx7

> > > > > Yes, I found other paths that miss the csi->is_csi2 check too.
  

Hi Rui,

On Tue, Jan 5, 2021 at 12:01 PM Rui Miguel Silva <rmfrfs@gmail.com> wrote:

> can you see if the following patch make it work again?

Yes, with your patch and mine I can capture the same way as with the
5.4 kernel :-)

The pink color issue is still present but it is orthogonal to this problem.

Could you please submit your patch formally to the list? Please
include my attached patch as 1/2 and yours as 2/2.

Also, please add the following tag to your patch:

Tested-by: Fabio Estevam <festevam@gmail.com>

Thanks,

Fabio Estevam
  

Oi Fabio,
On Tue, Jan 05, 2021 at 01:05:52PM -0300, Fabio Estevam wrote:
> Hi Rui,
> 
> On Tue, Jan 5, 2021 at 12:01 PM Rui Miguel Silva <rmfrfs@gmail.com>
> wrote:
> 
> > can you see if the following patch make it work again?
> 
> Yes, with your patch and mine I can capture the same way as with the
> 5.4 kernel :-)

Great that it worked.

> 
> The pink color issue is still present but it is orthogonal to this
> problem.
> 
> Could you please submit your patch formally to the list? Please
> include my attached patch as 1/2 and yours as 2/2.

yes, I will create a series with the correct fix tags also.

> 
> Also, please add the following tag to your patch:
> 
> Tested-by: Fabio Estevam <festevam@gmail.com>

will do, thanks and sorry about your issue, I really thought that all
imx, including imx5,6 and 7 had a mux. I need to get my hand in a
imx6ull full documentation.

------
Cheers,
     Rui
  

Hi Fabio,

On 1/5/21 2:19 PM, Fabio Estevam wrote:
> Hi Sébastien,
> 
> On Tue, Jan 5, 2021 at 6:49 AM Sébastien Szymanski
> <sebastien.szymanski@armadeus.com> wrote:
> 
>> I used the following gstreamer pipeline to stream on the framebuffer:
>>
>> gst-launch-1.0 v4l2src device=/dev/video1 ! v4l2convert ! fbdevsink
> 
> Thanks, this helps and now the pipeline starts and I do see the camera
> image on the LCD.
> 
> I switched to the same 5.4 you used just to make sure we are in the
> same codebase.
> 
> I am getting the wrong colors though. The captured image is too pinky.
> 
> Do you get the correct colors on your test?

Yes, the colors are correct on my test.

Regards,

> 
> Also, I had to describe like the polarities like this:
> 
> hsync-active = <0>;
> vsync-active = <1>;
> pclk-sample = <0>;
> 
> because if I used the same polarities from your patch, then the
> pipeline does not start.
> 
> Thanks for your help!
>
  

Hi Rui,

On 1/5/21 8:32 PM, Rui Miguel Silva wrote:
> Oi Fabio,
> On Tue, Jan 05, 2021 at 01:05:52PM -0300, Fabio Estevam wrote:
>> Hi Rui,
>>
>> On Tue, Jan 5, 2021 at 12:01 PM Rui Miguel Silva <rmfrfs@gmail.com>
>> wrote:
>>
>>> can you see if the following patch make it work again?
>>
>> Yes, with your patch and mine I can capture the same way as with the
>> 5.4 kernel :-)
> 
> Great that it worked.
> 
>>
>> The pink color issue is still present but it is orthogonal to this
>> problem.
>>
>> Could you please submit your patch formally to the list? Please
>> include my attached patch as 1/2 and yours as 2/2.
> 
> yes, I will create a series with the correct fix tags also.
> 
>>
>> Also, please add the following tag to your patch:
>>
>> Tested-by: Fabio Estevam <festevam@gmail.com>

Please add my tag too:

Tested-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>

Thanks!

Regards,


> 
> will do, thanks and sorry about your issue, I really thought that all
> imx, including imx5,6 and 7 had a mux. I need to get my hand in a
> imx6ull full documentation.
> 
> ------
> Cheers,
>      Rui
>