Hi Dorota, On Wed, 2021-10-06 at 13:05 +0200, Dorota Czaplejewicz wrote: > Section 13.7.6.13 "CSI Image Parameter Register" of the > i.MX 8M Quad Applications Processors Reference Manual > states that the line size should be divisible by 8 bytes. > However, the hardware also accepts sizes divisible by 4 bytes. > > This patch accepts line sizes divisible 4-bytes in non-planar mode. Thank you, this makes it much clearer. I see two issues with this, though, one small and one a bit bigger: First, I'd be wary of disregarding the reference manual - unless we know better, and then it should be well documented in the code. It might be that the 8-byte alignment requirement stems from the fact that the FIFO operates in double-word units, which might cause the CSI to write over the end of the buffer if the line width is odd (in 32-bit words). Or maybe it's just that the FBUF_STRIDE conflicts with this, I'm unclear on whether that is only given in units of dwords (although the driver currently doesn't support this anyway). I wonder: if you use 4-byte aligned width and odd height, does the CSI write over the end of the buffer? > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@xxxxxxx> > --- > > Hello, > > my previous patch identified something that was not a problem, > so I'm sending a different one. > > This has been tested on the Librem 5. > > Cheers, > Dorota > > drivers/staging/media/imx/imx-media-utils.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c > index 5128915a5d6f..a303003929e3 100644 > --- a/drivers/staging/media/imx/imx-media-utils.c > +++ b/drivers/staging/media/imx/imx-media-utils.c > @@ -545,13 +545,13 @@ int imx_media_mbus_fmt_to_pix_fmt(struct v4l2_pix_format *pix, > } > > /* Round up width for minimum burst size */ > - width = round_up(mbus->width, 8); > + width = round_up(mbus->width, 4); > > /* Round up stride for IDMAC line start address alignment */ > if (cc->planar) > stride = round_up(width, 16); > else > - stride = round_up((width * cc->bpp) >> 3, 8); > + stride = round_up((width * cc->bpp) >> 3, 4); Second, even if this works fine on the i.MX7/8M CSI, the alignment is still required for the i.MX5/6 IPU, for which this code and the comments were written. So we need a way to differentiate the two cases here. regards Philipp
