Re: media: imx: imx7-media-csi: Get rid of superfluous call to imx7_csi_mbus_fm t_to_pix_fmt

Tim Harvey
2023-07-19T17:28:37Z

On Mon, Jul 17, 2023 at 2:57 AM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Hi Tim,
>
> Am Freitag, 14. Juli 2023, 03:34:15 CEST schrieb Tim Harvey:
> > Alexander,
> >
> > I found that commit 6f482c4729d9: ("media: imx: imx7-media-csi: Get
> > rid of superfluous call to imx7_csi_mbus_fmt_to_pix_fmt") introduced
> > an issue causing me to not be able to capture anymore on an imx8mm
> > with an imx219 camera.
> >
> > I'm using a RaspberryPi Camera v2 which has an IMX219 8MP camera module:
> > - https://datasheets.raspberrypi.com/camera/camera-v2-schematics.pdf
> > - has its own on-board 24MHz osc so no clock required from baseboard
> > - pin 11 enables 1.8V and 2.8V LDO which is connected to a GPIO I use
> > as a regulator enable
> >
> > I'm using the imx8mm-venice-gw72xx-0x-imx219 dt overlay [1] to test this.
> >
> > Here is some additional information about how I'm using the camera module:
> > # cat /sys/bus/media/devices/media*/model
> > imx-media
> > hantro-vpu
> > hantro-vpu
> > # cat /sys/class/video4linux/video*/name
> > csi capture
> > nxp,imx8mm-vpu-g1-dec
> > nxp,imx8mq-vpu-g2-dec
> > # enable imx219 to csi link
> > media-ctl --reset
> > media-ctl -l "'imx219 2-0010':0 -> 'csis-32e30000.mipi-csi':0 [1]"
> > # configure for 640x480 raw8
> > media-ctl -v -V "'imx219 2-0010':0 [fmt:SRGGB8/640x480 field:none]"
> > media-ctl -v -V "'csis-32e30000.mipi-csi':0 [fmt:SRGGB8/640x480 field:none]"
> > media-ctl -v -V "'csi':0 [fmt:SRGGB8/640x480 field:none]"
> > # configure for RGGB (8-bit bayer), 640x480 resolution
> > v4l2-ctl --device /dev/video0
> > --set-fmt-video=width=640,height=480,pixelformat=RGGB --verbose
> >
> > before commit 6f482c4729d9: ("media: imx: imx7-media-csi: Get rid of
> > superfluous call to imx7_csi_mbus_fmt_to_pix_fmt") this would report
> > back 640x480 resolution:
> > VIDIOC_QUERYCAP: ok
> > VIDIOC_G_FMT: ok
> > VIDIOC_S_FMT: ok
> > Format Video Capture:
> > Width/Height : 640/480
> > Pixel Format : 'RGGB' (8-bit Bayer RGRG/GBGB)
> > Field : None
> > Bytes per Line : 640
> > Size Image : 307200
> > Colorspace : Default
> > Transfer Function : Default (maps to Rec. 709)
> > YCbCr/HSV Encoding: Default (maps to ITU-R 601)
> > Quantization : Default (maps to Full Range)
> > Flags :
> >
> > And after the commit it reports back an invalid 768x480 resolution:
> > VIDIOC_QUERYCAP: ok
> > VIDIOC_G_FMT: ok
> > VIDIOC_S_FMT: ok
> > Format Video Capture:
> > Width/Height : 768/480
> > Pixel Format : 'RGGB' (8-bit Bayer RGRG/GBGB)
> > Field : None
> > Bytes per Line : 768
> > Size Image : 368640
> > Colorspace : Default
> > Transfer Function : Default (maps to Rec. 709)
> > YCbCr/HSV Encoding: Default (maps to ITU-R 601)
> > Quantization : Default (maps to Full Range)
> > Flags :
> >
> > This resolution and frame size mis-match causes issues for example
> > when using gstreamer to capture and stream frames.
>
> Oh, that's weird. Can you check what the call to v4l_bound_align_image()
> inside __imx7_csi_video_try_fmt() is actually doing? Check walign, width
> before and after the call. From a glance that seems to be the only way width
> could be modified.
>

with debugging added in __imx7_csi_video_try_fmt:

root@jammy-venice:~# dmesg | grep csi
[ 0.038495] platform 32e30000.mipi-csi: Fixed dependency cycle(s)
with /soc@0/bus@32c00000/csi@32e20000/port/endpoint
[ 1.195055] __imx7_csi_video_try_fmt walign=4 bpp=16 640/480
[ 1.200746] __imx7_csi_video_try_fmt walign=4 bpp=16
bytesperline=1280 sizeimage=614400 640/480
[ 1.209633] imx7-csi 32e20000.csi: Registered csi capture as /dev/video0
[ 1.775321] i2c 2-0010: Fixed dependency cycle(s) with
/soc@0/bus@32c00000/mipi-csi@32e30000/ports/port@0/endpoint
[ 2.010372] imx-mipi-csis 32e30000.mipi-csi: lanes: 2, freq: 333000000
^^^ this is kernel init and looks fine

# media-ctl -l "'imx219 2-0010':0 -> 'csis-32e30000.mipi-csi':0 [1]"
# media-ctl -v -V "'imx219 2-0010':0 [fmt:SRGGB8/640x480 field:none]"
# media-ctl -v -V "'csis-32e30000.mipi-csi':0 [fmt:SRGGB8/640x480 field:none]"
# media-ctl -v -V "'csi':0 [fmt:SRGGB8/640x480 field:none]"
^^^ nothing changes here during link config (no calls to
__imx7_csi_video_try_fmt)

# v4l2-ctl --device /dev/video0
--set-fmt-video=width=640,height=480,pixelformat=RGGB --verbose
VIDIOC_QUERYCAP: ok
[ 120.367874] __imx7_csi_video_try_fmt walign=8 bpp=8 640/480
^^^ before calling v4l_bound_align_image()

VIDIOC_G_FMT: ok
[ 120.374974] __imx7_csi_video_try_fmt walign=8 bpp=8
bytesperline=768 sizeimage=368640 768/480
^^^ after calling v4l_bound_align_image() width is changed from 640 to 768

VIDIOC_S_FMT: ok
Format Video Capture:
Width/Height : 768/480
Pixel Format : 'RGGB' (8-bit Bayer RGRG/GBGB)
Field : None
Bytes per Line : 768
Size Image : 368640
Colorspace : Default
Transfer Function : Default (maps to Rec. 709)
YCbCr/HSV Encoding: Default (maps to ITU-R 601)
Quantization : Default (maps to Full Range)
Flags :

> > I noticed you have several outstanding patches pending for
> > imx7-media-csi... perhaps there is something there you already know of
> > that addresses this issue?
>
> There nothing pending AFAICS. Everything has been integrated into linux-next
> at least.

v6.5-rc2 shows the issue I've been discussing but linux-next
next-20230718 doesn't even probe mipi-csi and I'm not clear why yet:

with linux-next next-20230718:
# dmesg | grep csi
[ 1.172711] imx7-csi 32e20000.csi: Registered csi capture as /dev/video0
[ 1.179674] imx7-csi 32e20000.csi: error -ENOTCONN: Failed to get
remote endpoint
[ 1.187513] imx7-csi: probe of 32e20000.csi failed with error -107
[ 1.757693] i2c 2-0010: Fixed dependency cycle(s) with
/soc@0/bus@32c00000/mipi-csi@32e30000/ports/port@0/endpoint
[ 16.352048] platform 32e30000.mipi-csi: deferred probe pending
# cat /sys/kernel/debug/devices_deferred
32e30000.mipi-csi platform: wait for supplier
/soc@0/bus@32c00000/csi@32e20000/port/endpoint

In this tree both drivers/media/i2c/imx219.c and
drivers/media/platform/nxp/imx7-media-csi.c are at the same patch
level and I'm not sure what else is involved here to look at.

It seems mipi-dsi and mipi-csi are extremely fragile and require
constant regression testing unfortunately.

best regards,

Tim