[1/4] media: imx.rst: Fix the MIPI CSI-2 virtual channel

  The current instructions for imx6q-sabresd do not lead to functional
capture on OV5640 MIPI CSI-2.

The reason for this, as explained by Steve Longerbeam, is that OV5640 by
default transmits on virtual channel 0, not channel 1 as is given in the
instructions.

Adapt the instructions to use virtual channel 0 so that a working
camera setup can be achieved on imx6q-sabresd.

Suggested-by: Steve Longerbeam <slongerbeam@gmail.com>
Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
Hi Steve,

I don't have access to a imx6q sabrelite with camera, but I think
it needs fixing as well.

 Documentation/media/v4l-drivers/imx.rst | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)
  

Hi Fabio, thanks for the patch.

On 4/8/20 5:32 AM, Fabio Estevam wrote:
> The current instructions for imx6q-sabresd do not lead to functional
> capture on OV5640 MIPI CSI-2.
>
> The reason for this, as explained by Steve Longerbeam, is that OV5640 by
> default transmits on virtual channel 0, not channel 1 as is given in the
> instructions.
>
> Adapt the instructions to use virtual channel 0 so that a working
> camera setup can be achieved on imx6q-sabresd.
>
> Suggested-by: Steve Longerbeam <slongerbeam@gmail.com>
> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
> Hi Steve,
>
> I don't have access to a imx6q sabrelite with camera, but I think
> it needs fixing as well.
>
>   Documentation/media/v4l-drivers/imx.rst | 23 ++++++++++++-----------
>   1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/media/v4l-drivers/imx.rst b/Documentation/media/v4l-drivers/imx.rst
> index 1246573c1019..7c3afd32e456 100644
> --- a/Documentation/media/v4l-drivers/imx.rst
> +++ b/Documentation/media/v4l-drivers/imx.rst
> @@ -645,25 +645,26 @@ The OV5640 module connects to MIPI connector J5 (sorry I don't have the
>   compatible module part number or URL).
>   
>   The following example configures a direct conversion pipeline to capture
> -from the OV5640, transmitting on MIPI CSI-2 virtual channel 1. $sensorfmt
> -can be any format supported by the OV5640. $sensordim is the frame
> -dimension part of $sensorfmt (minus the mbus pixel code). $outputfmt can
> -be any format supported by the ipu1_ic_prpenc entity at its output pad:
> +from the OV5640, transmitting on MIPI CSI-2 virtual channel 0:

Since this is an Image Converter pipeline, it would be nice to 
demonstrate scaling and color conversion in this example. How about 
changing the format at IC output to be ARGB8888_1X32/800x600. So change 
the explanation of the pipeline above, and:

>   
>   .. code-block:: none
>   
>      # Setup links
>      media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
> -   media-ctl -l "'imx6-mipi-csi2':2 -> 'ipu1_csi1':0[1]"
> -   media-ctl -l "'ipu1_csi1':1 -> 'ipu1_ic_prp':0[1]"
> +   media-ctl -l "'imx6-mipi-csi2':1 -> 'ipu1_csi0_mux':0[1]"
> +   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
> +   media-ctl -l "'ipu1_csi0':1 -> 'ipu1_ic_prp':0[1]"
>      media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
>      media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
>      # Configure pads
> -   media-ctl -V "'ov5640 1-003c':0 [fmt:$sensorfmt field:none]"
> -   media-ctl -V "'imx6-mipi-csi2':2 [fmt:$sensorfmt field:none]"
> -   media-ctl -V "'ipu1_csi1':1 [fmt:AYUV32/$sensordim field:none]"
> -   media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/$sensordim field:none]"
> -   media-ctl -V "'ipu1_ic_prpenc':1 [fmt:$outputfmt field:none]"
> +   media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "'imx6-mipi-csi2':1 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/640x480]"
> +   media-ctl -V "'ipu1_csi0':0 [fmt:UYVY2X8/640x480]"

don't need this line (I pulled this from a script that was testing 
crop/compose on sink pad ipu1_csi0:0, but since we're not doing that 
here, you can remove the format setup line for ipu1_csi0:0).

> +   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/640x480]"
> +   media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480]"
> +   media-ctl -V "'ipu1_ic_prpenc':0 [fmt:AYUV32/640x480]"

don't need this line.

> +   media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/640x480]"

change to:

media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"


>   
>   Streaming can then begin on "ipu1_ic_prpenc capture" node. The v4l2-ctl
>   tool can be used to select any supported YUV or RGB pixelformat on the

"...The v4l2-ctl tool can be used to select any supported RGB pixelformat..."


Steve
  

Hi Steve,

On Wed, Apr 8, 2020 at 1:35 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> change to:
>
> media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"

If I make this change I get:

# v4l2-ctl --stream-mmap -d /dev/video1
[  501.143012] ipu1_ic_prpenc: capture format not valid
                VIDIOC_STREAMON returned -1 (Invalid argument)

Going back to the original example:

#  media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/640x480]"
# v4l2-ctl --stream-mmap -d /dev/video1
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps

Changing only the resolution:

#  media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/800x600]"
# v4l2-ctl --stream-mmap -d /dev/video1
[  658.358246] ipu1_ic_prpenc: capture format not valid
                VIDIOC_STREAMON returned -1 (Invalid argument)

Any ideas?

These are the lines I am currently using as per your feedback:

   # Setup links
   media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
   media-ctl -l "'imx6-mipi-csi2':1 -> 'ipu1_csi0_mux':0[1]"
   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
   media-ctl -l "'ipu1_csi0':1 -> 'ipu1_ic_prp':0[1]"
   media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
   media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
   # Configure pads
   media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'imx6-mipi-csi2':1 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/640x480]"
   media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480]"
   media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"

Thanks,

Fabio Estevam

   # Setup links
   media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
   media-ctl -l "'imx6-mipi-csi2':1 -> 'ipu1_csi0_mux':0[1]"
   media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
   media-ctl -l "'ipu1_csi0':1 -> 'ipu1_ic_prp':0[1]"
   media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
   media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
   # Configure pads
   media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'imx6-mipi-csi2':1 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/640x480]"
   media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/640x480]"
   media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480]"
   media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"
  

On 4/8/20 10:24 AM, Fabio Estevam wrote:
> Hi Steve,
>
> On Wed, Apr 8, 2020 at 1:35 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>> change to:
>>
>> media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"
> If I make this change I get:
>
> # v4l2-ctl --stream-mmap -d /dev/video1
> [  501.143012] ipu1_ic_prpenc: capture format not valid
>                  VIDIOC_STREAMON returned -1 (Invalid argument)
>
> Going back to the original example:
>
> #  media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/640x480]"
> # v4l2-ctl --stream-mmap -d /dev/video1
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
>
> Changing only the resolution:
>
> #  media-ctl -V "'ipu1_ic_prpenc':1 [fmt:AYUV32/800x600]"
> # v4l2-ctl --stream-mmap -d /dev/video1
> [  658.358246] ipu1_ic_prpenc: capture format not valid
>                  VIDIOC_STREAMON returned -1 (Invalid argument)
>
> Any ideas?

You have to set a format at the capture interface as the last step 
before streaming can start:

# v4l2-ctl -d /dev/video1 --set-fmt-video=pixelformat=RGB3


or whatever RGB format is supported. It's enough to sinmply set one parameter, such as width or height or pixelformat. The driver will propagate the reset from the source subdev ipu1_ic_prpenc.

The capture interface used to be automatically propagated from the source subdev, but now it must be done by userland.

Steve

>
> These are the lines I am currently using as per your feedback:
>
>     # Setup links
>     media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
>     media-ctl -l "'imx6-mipi-csi2':1 -> 'ipu1_csi0_mux':0[1]"
>     media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>     media-ctl -l "'ipu1_csi0':1 -> 'ipu1_ic_prp':0[1]"
>     media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
>     media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
>     # Configure pads
>     media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'imx6-mipi-csi2':1 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/640x480]"
>     media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480]"
>     media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"
>
> Thanks,
>
> Fabio Estevam
>
>     # Setup links
>     media-ctl -l "'ov5640 1-003c':0 -> 'imx6-mipi-csi2':0[1]"
>     media-ctl -l "'imx6-mipi-csi2':1 -> 'ipu1_csi0_mux':0[1]"
>     media-ctl -l "'ipu1_csi0_mux':2 -> 'ipu1_csi0':0[1]"
>     media-ctl -l "'ipu1_csi0':1 -> 'ipu1_ic_prp':0[1]"
>     media-ctl -l "'ipu1_ic_prp':1 -> 'ipu1_ic_prpenc':0[1]"
>     media-ctl -l "'ipu1_ic_prpenc':1 -> 'ipu1_ic_prpenc capture':0[1]"
>     # Configure pads
>     media-ctl -V "'ov5640 1-003c':0 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'imx6-mipi-csi2':1 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'ipu1_csi0_mux':2 [fmt:UYVY2X8/640x480]"
>     media-ctl -V "'ipu1_csi0':1 [fmt:AYUV32/640x480]"
>     media-ctl -V "'ipu1_ic_prp':1 [fmt:AYUV32/640x480]"
>     media-ctl -V "'ipu1_ic_prpenc':1 [fmt:ARGB8888_1X32/800x600]"
  

Hi Steve,

On Wed, Apr 8, 2020 at 4:16 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:

> You have to set a format at the capture interface as the last step
> before streaming can start:
>
> # v4l2-ctl -d /dev/video1 --set-fmt-video=pixelformat=RGB3

Thanks. After running this line the stream can start via --stream-mmap:

# v4l2-ctl --stream-mmap -d /dev/video1
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps

but it fails with Gstreamer:

# gst-launch-1.0 -v v4l2src device=/dev/video1 ! kmssink
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 1024
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 768
ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
Internal data stream error.
Additional debug info:
../libs/gst/base/gstbasesrc.c(3072): gst_base_src_loop ():
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
streaming stopped, reason not-negotiated (-4)
ERROR: pipeline doesn't want to preroll.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...

Should I change the Gstreamer pipeline?

Thanks
  

On Wed, Apr 8, 2020 at 4:28 PM Fabio Estevam <festevam@gmail.com> wrote:

> Should I change the Gstreamer pipeline?

It works if I use the v4l2convert element:

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

I will send it in the next patch series.

Thanks

>
> Thanks
  

On 4/8/20 12:28 PM, Fabio Estevam wrote:
> Hi Steve,
>
> On Wed, Apr 8, 2020 at 4:16 PM Steve Longerbeam <slongerbeam@gmail.com> wrote:
>
>> You have to set a format at the capture interface as the last step
>> before streaming can start:
>>
>> # v4l2-ctl -d /dev/video1 --set-fmt-video=pixelformat=RGB3
> Thanks. After running this line the stream can start via --stream-mmap:
>
> # v4l2-ctl --stream-mmap -d /dev/video1
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
> <<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.03 fps
>
> but it fails with Gstreamer:
>
> # gst-launch-1.0 -v v4l2src device=/dev/video1 ! kmssink
> Setting pipeline to PAUSED ...
> Pipeline is live and does not need PREROLL ...
> /GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 1024
> /GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 768
> ERROR: from element /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> Internal data stream error.
> Additional debug info:
> ../libs/gst/base/gstbasesrc.c(3072): gst_base_src_loop ():
> /GstPipeline:pipeline0/GstV4l2Src:v4l2src0:
> streaming stopped, reason not-negotiated (-4)
> ERROR: pipeline doesn't want to preroll.
> Setting pipeline to PAUSED ...
> Setting pipeline to READY ...
> Setting pipeline to NULL ...
> Freeing pipeline ...
>
> Should I change the Gstreamer pipeline?

It works for me, I'm running gstreamer version:

arm:~/scripts % gst-launch-1.0 --version
gst-launch-1.0 version 1.14.5
GStreamer 1.14.5
https://launchpad.net/distros/ubuntu/+source/gstreamer1.0

Also, I'm streaming to an HDMI monitor at 1920x1080 (passing 
connector-id=54 to kmssink).

I can't tell from your gst output whether you are having an issue with 
pixelformats or frame dimensions, but anyway here is my gst pipeline:

gst-launch-1.0 v4l2src device=/dev/video1 ! kmssink connector-id=54 
name=imx-drm sync=0 can-scale=false

It might also be the can-scale property, you might need to provide 
can-scale=false to tell kmssink that imx-drm cannot scale 800x600 to 
something else.

While streaming, I checked what pixelformat the pipeline finally settled on:

arm:~/scripts % v4l2-ctl -d1 -V
Format Video Capture:
     Width/Height      : 800/600
     Pixel Format      : 'XR24'
     Field             : None
     Bytes per Line    : 3200
     Size Image        : 1920000
     Colorspace        : sRGB
     Transfer Function : sRGB
     YCbCr/HSV Encoding: ITU-R 601
     Quantization      : Full Range
     Flags             :

Steve
  

On 4/8/20 12:33 PM, Fabio Estevam wrote:
> On Wed, Apr 8, 2020 at 4:28 PM Fabio Estevam <festevam@gmail.com> wrote:
>
>> Should I change the Gstreamer pipeline?
> It works if I use the v4l2convert element:
>
> gst-launch-1.0 -v v4l2src device=/dev/video1 ! v4l2convert ! kmssink

I think that works because the csc-scaler mem2mem driver is able to 
provide the frame dimensions that kmssink requires, but I don't think 
you need v4l2convert, I think this should work (use can-scale=false):

gst-launch-1.0 v4l2src device=/dev/video1 ! kmssink connector-id=54 
name=imx-drm sync=0 can-scale=false

but use the connector-id for your display.

Steve
  

Hi Fabio,

So sorry!

I was testing with the patchset:

[PATCH v7 00/11] media: imx: Miscellaneous format-related cleanups

which specifically fixes a bug with enumerating non-mbus RGB formats:

[PATCH v7 01/11] media: imx: utils: fix and simplify pixel format 
enumeration

Without that patchset, /dev/video1 only lists:

arm:~/scripts % v4l2-ctl -d1 --list-formats
ioctl: VIDIOC_ENUM_FMT
     Index       : 0
     Type        : Video Capture
     Pixel Format: 'RGBP'
     Name        : 16-bit RGB 5-6-5

     Index       : 1
     Type        : Video Capture
     Pixel Format: 'RGB3'
     Name        : 24-bit RGB 8-8-8

     Index       : 2
     Type        : Video Capture
     Pixel Format: 'BX24'
     Name        : 32-bit XRGB 8-8-8-8

But with the bug fix:

arm:~ % v4l2-ctl -d1 --list-formats
ioctl: VIDIOC_ENUM_FMT
         Index       : 0
         Type        : Video Capture
         Pixel Format: 'RGBP'
         Name        : 16-bit RGB 5-6-5

         Index       : 1
         Type        : Video Capture
         Pixel Format: 'RGB3'
         Name        : 24-bit RGB 8-8-8

         Index       : 2
         Type        : Video Capture
         Pixel Format: 'BGR3'
         Name        : 24-bit BGR 8-8-8

         Index       : 3
         Type        : Video Capture
         Pixel Format: 'BX24'
         Name        : 32-bit XRGB 8-8-8-8

         Index       : 4
         Type        : Video Capture
         Pixel Format: 'XR24'
         Name        : 32-bit BGRX 8-8-8-8

         Index       : 5
         Type        : Video Capture
         Pixel Format: 'RX24'
         Name        : 32-bit XBGR 8-8-8-8

         Index       : 6
         Type        : Video Capture
         Pixel Format: 'XB24'
         Name        : 32-bit RGBX 8-8-8-8


And with that, kmssink is able to negotiate XR24.

With the anticipation that the patchset mentioned will be merged, can 
you try again with that patchset applied, and update the imx.rst doc 
accordingly?

Thanks,
Steve


On 4/8/20 12:51 PM, Steve Longerbeam wrote:
>
>
> On 4/8/20 12:33 PM, Fabio Estevam wrote:
>> On Wed, Apr 8, 2020 at 4:28 PM Fabio Estevam <festevam@gmail.com> wrote:
>>
>>> Should I change the Gstreamer pipeline?
>> It works if I use the v4l2convert element:
>>
>> gst-launch-1.0 -v v4l2src device=/dev/video1 ! v4l2convert ! kmssink
>
> I think that works because the csc-scaler mem2mem driver is able to 
> provide the frame dimensions that kmssink requires, but I don't think 
> you need v4l2convert, I think this should work (use can-scale=false):
>
> gst-launch-1.0 v4l2src device=/dev/video1 ! kmssink connector-id=54 
> name=imx-drm sync=0 can-scale=false
>
> but use the connector-id for your display.
>
> Steve
>