media: imx: imx7-media-csi: Get rid of superfluous call to imx7_csi_mbus_fm t_to_pix_fmt
Alexander Stein
alexander.stein at ew.tq-group.com
Wed Jul 19 23:49:00 PDT 2023
Hi Tim,
Am Mittwoch, 19. Juli 2023, 19:27:54 CEST schrieb Tim Harvey:
> On Mon, Jul 17, 2023 at 2:57 AM Alexander Stein
>
> <alexander.stein at 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 at jammy-venice:~# dmesg | grep csi
> [ 0.038495] platform 32e30000.mipi-csi: Fixed dependency cycle(s)
> with /soc at 0/bus at 32c00000/csi at 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 at 0/bus at 32c00000/mipi-csi at 32e30000/ports/port at 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
So this call is the culprit, I noticed the important change to before. The
alignment enforced in clamp_align(), called by v4l_bound_align_image(), is a
multiple of 2^align, _not_ a multiple of align. So obviously 640 is increased
to the next multiple of 2^8 (256) which is 768. Can you give the following
diff a try?
--8<--
diff --git a/drivers/media/platform/nxp/imx7-media-csi.c b/drivers/media/
platform/nxp/imx7-media-csi.c
index 2f9302fc7570..a040665eca7c 100644
--- a/drivers/media/platform/nxp/imx7-media-csi.c
+++ b/drivers/media/platform/nxp/imx7-media-csi.c
@@ -1137,9 +1137,8 @@ __imx7_csi_video_try_fmt(struct v4l2_pix_format *pixfmt,
* TODO: Implement configurable stride support.
*/
walign = 8 * 8 / cc->bpp;
- v4l_bound_align_image(&pixfmt->width, 1, 0xffff, walign,
- &pixfmt->height, 1, 0xffff, 1, 0);
-
+ pixfmt->width = clamp_roundup(pixfmt->width, 1, 0xffff, walign);
+ pixfmt->height = clamp_roundup(pixfmt->width, 1, 0xffff, 1);
pixfmt->bytesperline = pixfmt->width * cc->bpp / 8;
pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
pixfmt->field = V4L2_FIELD_NONE;
--8<--
Thanks and best regards,
Alexander
> 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 at 0/bus at 32c00000/mipi-csi at 32e30000/ports/port at 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 at 0/bus at 32c00000/csi at 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
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
More information about the linux-arm-kernel
mailing list