| Message ID | 20230725200249.15447-2-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx: imx7-media-csi: Fix width alignment | expand |
Hi Laurent, thanks for picking this up. Am Dienstag, 25. Juli 2023, 22:02:47 CEST schrieb Laurent Pinchart: > From: Fabio Estevam <festevam@denx.de> > > 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@gateworks.com> > Closes: > https://lore.kernel.org/linux-media/CAJ+vNU0BOVLTL17ofgHwtexbpuMYwH_aGUC==E > XABUtHHiv_ag@mail.gmail.com 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@ew.tq-group.com> Signed-off-by: Alexander Stein > <alexander.stein@ew.tq-group.com> > Signed-off-by: Fabio Estevam <festevam@denx.de> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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@ew.tq-group.com> Thanks Alexander > > pixfmt->bytesperline = pixfmt->width * cc->bpp / 8; > pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
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); pixfmt->bytesperline = pixfmt->width * cc->bpp / 8; pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;