[PATCH 1/2] media: imx: Round line size to 4 bytes

[PATCH 1/2] media: imx: Round line size to 4 bytes

[Dorota Czaplejewicz]

[-- Attachment #1.1: Type: text/plain, Size: 2412 bytes --]

This has been broken in e352833d, which does not actually copy the logic
from 056f9af9.

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.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
---
Hi,

in recent kernels, I've noticed that the Librem 5 camera driver no longer works. The s5k3l6xx out of tree sensor driver uses sizes divisible by 4 and not by 8, for which I submitted the patch 056f9af9 . The new kernels no longer accept sizes divisible by 4.

I tracked down the source: previously, the video device on the imx8m used imx_media_mbus_fmt_to_pix_fmt. e352833d introduced imx7_csi_mbus_fmt_to_pix_fmt, which is what is used now exclusively. However, imx7_csi_mbus_fmt_to_pix_fmt *does not* contain the same logic, and does not provide an explanation why the logic is different.

I don't know why the "imx_*" is not used any more on the i.MX8M SoC, but I presume that "imx7_*" is the correct code path. That indicates that "imx_*" is *not* the correct code for this SoC.

Under those assumptions, I added the relaxed rounding to the "imx7_*" function, to match what the SoC can do (patch 1). I also reverted my original patch (patch 2), because if "imx7_*" is the correct function for my SoC, then I had never intended to mess with whatever other SoC is handled by the "imx_" function.

Cheers,
Dorota

 drivers/staging/media/imx/imx7-media-csi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx7-media-csi.c b/drivers/staging/media/imx/imx7-media-csi.c
index 643547bfa90f..bafbb5ef08d5 100644
--- a/drivers/staging/media/imx/imx7-media-csi.c
+++ b/drivers/staging/media/imx/imx7-media-csi.c
@@ -1000,10 +1000,10 @@ static int imx7_csi_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 */
-	stride = round_up((width * cc->bpp) >> 3, 8);
+	stride = round_up((width * cc->bpp) >> 3, 4);
 
 	pix->width = width;
 	pix->height = mbus->height;
-- 
2.37.3


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] Revert "media: imx: Round line size to 4 bytes"

[Dorota Czaplejewicz]

[-- Attachment #1.1: Type: text/plain, Size: 1175 bytes --]

This reverts commit 056f9af9fbebb81ebc972540122fb3bdad81d8b1.

The method is no longer getting called on the i.MX8, meaning that
the responsibility for this functionality has been moved entirely to
imx7_csi_mbus_fmt_to_pix_fmt.

Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
---
 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 73cf0d855967..294c808b2ebe 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, 4);
+	width = round_up(mbus->width, 8);
 
 	/* 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, 4);
+		stride = round_up((width * cc->bpp) >> 3, 8);
 
 	pix->width = width;
 	pix->height = mbus->height;
-- 
2.37.3


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] media: imx: Round line size to 4 bytes

[Dan Carpenter]

On Wed, Sep 14, 2022 at 01:58:08PM +0200, Dorota Czaplejewicz wrote:
> This has been broken in e352833d, which does not actually copy the logic
> from 056f9af9.
> 

There is no commit 056f9af9.  Always the human readable subject for the
patch.  Also use 12 characters and not 8 so we don't get two commits
with the same hash.

Please add a Fixes tag.
Fixes: e352833d32e6 ("media: staging: media: imx: imx7-media-csi: Import format helpers")

A Fixes tag can be used to automatically determine if a patch needs a
backport or not.  It's just useful information as well.

> 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.
> 
> Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> ---
> Hi,
> 
> in recent kernels, I've noticed that the Librem 5 camera driver no longer works. The s5k3l6xx out of tree sensor driver uses sizes divisible by 4 and not by 8, for which I submitted the patch 056f9af9 . The new kernels no longer accept sizes divisible by 4.
> 

This information needs to be in the commit message.  Otherwise the
commit message just sounds like theory and abstraction which we do not
care about.  We do care about real hardware which is not working and
especially if it used to work.

I'm not really qualified to review the details of this patch.
Laurent is best qualified to review this.  He'll get this message
because he's subscribed to the lists but it's also good to CC him as
well because he's probably drowning in email like the rest of us.  I've
added him.

regards,
dan carpenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] media: imx: Round line size to 4 bytes

[Dorota Czaplejewicz]

[-- Attachment #1.1: Type: text/plain, Size: 2174 bytes --]

Hi,

On Wed, 14 Sep 2022 17:29:48 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Wed, Sep 14, 2022 at 01:58:08PM +0200, Dorota Czaplejewicz wrote:
> > This has been broken in e352833d, which does not actually copy the logic
> > from 056f9af9.
> >   
> 
> There is no commit 056f9af9.  Always the human readable subject for the
> patch.  Also use 12 characters and not 8 so we don't get two commits
> with the same hash.

Thanks. Unbeknownst to me, no one ever applied or rejected the patch series I sent and saw under 056f9af9.

I'll send a fresh series in a moment.

Cheers
--Dorota
> 
> Please add a Fixes tag.
> Fixes: e352833d32e6 ("media: staging: media: imx: imx7-media-csi: Import format helpers")
> 
> A Fixes tag can be used to automatically determine if a patch needs a
> backport or not.  It's just useful information as well.
> 
> > 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.
> > 
> > Signed-off-by: Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm>
> > ---
> > Hi,
> > 
> > in recent kernels, I've noticed that the Librem 5 camera driver no longer works. The s5k3l6xx out of tree sensor driver uses sizes divisible by 4 and not by 8, for which I submitted the patch 056f9af9 . The new kernels no longer accept sizes divisible by 4.
> >   
> 
> This information needs to be in the commit message.  Otherwise the
> commit message just sounds like theory and abstraction which we do not
> care about.  We do care about real hardware which is not working and
> especially if it used to work.
> 
> I'm not really qualified to review the details of this patch.
> Laurent is best qualified to review this.  He'll get this message
> because he's subscribed to the lists but it's also good to CC him as
> well because he's probably drowning in email like the rest of us.  I've
> added him.
> 
> regards,
> dan carpenter
> 


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel