Hi Laurent, thanks for picking this up. Am Dienstag, 25. Juli 2023, 22:02:47 CEST schrieb Laurent Pinchart: > From: Fabio Estevam <festevam@xxxxxxx> > > v4l_bound_align_image() aligns to a multiple of 2 to the power of > walign, not to walign. Depending on the pixel format, this causes the > image width to be aligned to 16 or 256 pixels instead of 4 or 8 as > required by the hardware. Fix it by rounding and clamping the width and > height manually. > > Reported-by: Tim Harvey <tharvey@xxxxxxxxxxxxx> > Closes: > https://lore.kernel.org/linux-media/CAJ+vNU0BOVLTL17ofgHwtexbpuMYwH_aGUC==E > XABUtHHiv_ag@xxxxxxxxxxxxxx Fixes: 6f482c4729d9 ("media: imx: > imx7-media-csi: Get rid of superfluous call to > imx7_csi_mbus_fmt_to_pix_fmt") Co-developed-by: Alexander Stein > <alexander.stein@xxxxxxxxxxxxxxx> Signed-off-by: Alexander Stein > <alexander.stein@xxxxxxxxxxxxxxx> > Signed-off-by: Fabio Estevam <festevam@xxxxxxx> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > --- > Changes since v2: > > - Don't export clamp_roundup() from v4l2-common.c > - Simply clamp the height as no alignment is needed > - Add required includes > --- > drivers/media/platform/nxp/imx7-media-csi.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c > b/drivers/media/platform/nxp/imx7-media-csi.c index > 2ec1f3cd56a0..5684ecd2e3fe 100644 > --- a/drivers/media/platform/nxp/imx7-media-csi.c > +++ b/drivers/media/platform/nxp/imx7-media-csi.c > @@ -9,7 +9,9 @@ > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/interrupt.h> > +#include <linux/math.h> > #include <linux/mfd/syscon.h> > +#include <linux/minmax.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_graph.h> > @@ -1137,8 +1139,9 @@ __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(round_up(pixfmt->width, walign), walign, > + round_down(65535U, walign)); > + pixfmt->height = clamp(pixfmt->height, 1U, 65535U); Actually I have a slight preference for 0xffff over 65535, just because it is indicating this is some (maximum) register value. But it may just be me. I'm okay either way: Reviewed-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> Thanks Alexander > > pixfmt->bytesperline = pixfmt->width * cc->bpp / 8; > pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height; -- 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/
