Change rounding to the minimal burst size from 2^n to n.
This fixes images with sizes that are a multiple of 8 pixels.
See section 13.7.6.13 CSI Image Parameter Register of the
i.MX 8M Quad Applications Processors Reference Manual.
Fixes: 451a7b7815d0b ("media: imx: lift CSI and PRP ENC/VF width
alignment restriction")
Signed-off-by: Dorota Czaplejewicz <[email protected]>
---
Hi,
I tested this patch on the Librem 5 with the main camera.
--Dorota
drivers/staging/media/imx/imx-media-utils.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
index 5128915a5d6f..a2d8fab32a39 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, 3);
/* Round up stride for IDMAC line start address alignment */
if (cc->planar)
- stride = round_up(width, 16);
+ stride = round_up(width, 4);
else
- stride = round_up((width * cc->bpp) >> 3, 8);
+ stride = round_up((width * cc->bpp) >> 3, 3);
pix->width = width;
pix->height = mbus->height;
--
2.31.1
Hi,
On Fri, 24 Sep 2021 13:46:15 +0200
Philipp Zabel <[email protected]> wrote:
> Hi Dorota,
>
> On Fri, Sep 24, 2021 at 12:06:31PM +0200, Dorota Czaplejewicz wrote:
> > Change rounding to the minimal burst size from 2^n to n.
> >
> > This fixes images with sizes that are a multiple of 8 pixels.
>
> Could you elaborate on what is currently wrong with images that are a
> multiple of 8 pixels wide? Or are you rather trying to add support
> for images that are not a multiple of 8 pixels wide?
>
> >
> > See section 13.7.6.13 CSI Image Parameter Register of the
> > i.MX 8M Quad Applications Processors Reference Manual.
> >
> > Fixes: 451a7b7815d0b ("media: imx: lift CSI and PRP ENC/VF width
> > alignment restriction")
> > Signed-off-by: Dorota Czaplejewicz <[email protected]>
> > ---
> > Hi,
> >
> > I tested this patch on the Librem 5 with the main camera.
> >
> > --Dorota
> > drivers/staging/media/imx/imx-media-utils.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> > index 5128915a5d6f..a2d8fab32a39 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, 3);
>
> That is not a valid use of the round_up() macro anymore.
> The second parameter must be a power of 2.
>
> regards
> Philipp
Thanks for catching that. It turns out I was fixing an image that was a multiple of 4, but not a multiple of 8.
The problem that occurred has to do with an image of size 1052x780, which covers the entire sensor in use at 1/4 scale.
The sensor was configured to emit 1052*780 8-bit pixels, whereas the CSI portion here rounded up the image, expecting 1056*780. As a result, the userspace received a buffer sized according to the latter, each frame padded with some pixels from the next. As a result, each frame was composed of two or more, with a broken TV effect.
The data sheet says that the width parameter "should" be a multiple of 8:
> If the input data from the sensor is 8-bit/pixel format, the IMAGE_WIDTH should be a multiple of 8 pixels.
Nevertheless, I double-tested this, and changing the relevant rounding from "8" to "4" works flawlessly here. Lower divisors were tested previously and did not work.
Does it make sense to accept 4 as a valid divisor, given that rounding is likely not going to help but result in an invalid image anyway (as it did for me)?
Cheers,
Dorota
Hi Dorota,
On Fri, Sep 24, 2021 at 12:06:31PM +0200, Dorota Czaplejewicz wrote:
> Change rounding to the minimal burst size from 2^n to n.
>
> This fixes images with sizes that are a multiple of 8 pixels.
Could you elaborate on what is currently wrong with images that are a
multiple of 8 pixels wide? Or are you rather trying to add support
for images that are not a multiple of 8 pixels wide?
>
> See section 13.7.6.13 CSI Image Parameter Register of the
> i.MX 8M Quad Applications Processors Reference Manual.
>
> Fixes: 451a7b7815d0b ("media: imx: lift CSI and PRP ENC/VF width
> alignment restriction")
> Signed-off-by: Dorota Czaplejewicz <[email protected]>
> ---
> Hi,
>
> I tested this patch on the Librem 5 with the main camera.
>
> --Dorota
> drivers/staging/media/imx/imx-media-utils.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx-media-utils.c b/drivers/staging/media/imx/imx-media-utils.c
> index 5128915a5d6f..a2d8fab32a39 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, 3);
That is not a valid use of the round_up() macro anymore.
The second parameter must be a power of 2.
regards
Philipp