diff mbox series

[1/7] media: sun6i-csi: Fix the bpp for 10-bit bayer formats

Message ID 20200821145935.20346-2-kevin.lhopital@bootlin.com (mailing list archive)
State New, archived
Headers show
Series Support of MIPI CSI-2 for A83T and OV8865 camera | expand

Commit Message

Kévin L'hôpital Aug. 21, 2020, 2:59 p.m. UTC
10-bit bayer formats are aligned to 16 bits in memory, so this is what
needs to be used as bpp for calculating the size of the buffers to
allocate.

Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
---
 drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Maxime Ripard Aug. 24, 2020, 4:55 p.m. UTC | #1
On Fri, Aug 21, 2020 at 04:59:29PM +0200, Kévin L'hôpital wrote:
> 10-bit bayer formats are aligned to 16 bits in memory, so this is what
> needs to be used as bpp for calculating the size of the buffers to
> allocate.
> 
> Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>

Generally speaking, you should also explain why it's not an issue for
the callers. Depending on what that function is supposed to be doing
(returning the padded bits or the padded bits per pixel), your patch
could be either right or wrong.

Since all the callers are using it to generate the number of bytes per
line, your patch is indeed correct. But it should be mentionned in the
commit log.

Maxime
Kévin L'hôpital Aug. 25, 2020, 3:23 p.m. UTC | #2
Hello,

Le Mon, 24 Aug 2020 18:55:36 +0200,
Maxime Ripard <maxime@cerno.tech> a écrit :

> On Fri, Aug 21, 2020 at 04:59:29PM +0200, Kévin L'hôpital wrote:
> > 10-bit bayer formats are aligned to 16 bits in memory, so this is
> > what needs to be used as bpp for calculating the size of the
> > buffers to allocate.
> > 
> > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>  
> 
> Generally speaking, you should also explain why it's not an issue for
> the callers. Depending on what that function is supposed to be doing
> (returning the padded bits or the padded bits per pixel), your patch
> could be either right or wrong.
> 
> Since all the callers are using it to generate the number of bytes per
> line, your patch is indeed correct. But it should be mentionned in the
> commit log.
> 
> Maxime

All right, I will add this explanation.

Thank you very much for the review

Kévin
Chen-Yu Tsai Aug. 25, 2020, 3:50 p.m. UTC | #3
On Fri, Aug 21, 2020 at 11:00 PM Kévin L'hôpital
<kevin.lhopital@bootlin.com> wrote:
>
> 10-bit bayer formats are aligned to 16 bits in memory, so this is what
> needs to be used as bpp for calculating the size of the buffers to
> allocate.
>
> Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>

Please add:

Fixes: 5cc7522d8965 ("media: sun6i: Add support for Allwinner CSI V3s")



> ---
>  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> index c626821aaedb..8b83d15de0d0 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> @@ -100,7 +100,7 @@ static inline int sun6i_csi_get_bpp(unsigned int pixformat)
>         case V4L2_PIX_FMT_SGBRG10:
>         case V4L2_PIX_FMT_SGRBG10:
>         case V4L2_PIX_FMT_SRGGB10:
> -               return 10;
> +               return 16;
>         case V4L2_PIX_FMT_SBGGR12:
>         case V4L2_PIX_FMT_SGBRG12:
>         case V4L2_PIX_FMT_SGRBG12:
> --
> 2.17.1
>
Kévin L'hôpital Aug. 26, 2020, 9:18 a.m. UTC | #4
Hello,

Le Tue, 25 Aug 2020 23:50:30 +0800,
Chen-Yu Tsai <wens@csie.org> a écrit :

> On Fri, Aug 21, 2020 at 11:00 PM Kévin L'hôpital
> <kevin.lhopital@bootlin.com> wrote:
> >
> > 10-bit bayer formats are aligned to 16 bits in memory, so this is
> > what needs to be used as bpp for calculating the size of the
> > buffers to allocate.
> >
> > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>  
> 
> Please add:
> 
> Fixes: 5cc7522d8965 ("media: sun6i: Add support for Allwinner CSI
> V3s")
> 
> 
I will add this, thank you very much for the review.
Kévin
> 
> > ---
> >  drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h index
> > c626821aaedb..8b83d15de0d0 100644 ---
> > a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h +++
> > b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h @@ -100,7
> > +100,7 @@ static inline int sun6i_csi_get_bpp(unsigned int
> > pixformat) case V4L2_PIX_FMT_SGBRG10: case V4L2_PIX_FMT_SGRBG10:
> >         case V4L2_PIX_FMT_SRGGB10:
> > -               return 10;
> > +               return 16;
> >         case V4L2_PIX_FMT_SBGGR12:
> >         case V4L2_PIX_FMT_SGBRG12:
> >         case V4L2_PIX_FMT_SGRBG12:
> > --
> > 2.17.1
> >
diff mbox series

Patch

diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
index c626821aaedb..8b83d15de0d0 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
@@ -100,7 +100,7 @@  static inline int sun6i_csi_get_bpp(unsigned int pixformat)
 	case V4L2_PIX_FMT_SGBRG10:
 	case V4L2_PIX_FMT_SGRBG10:
 	case V4L2_PIX_FMT_SRGGB10:
-		return 10;
+		return 16;
 	case V4L2_PIX_FMT_SBGGR12:
 	case V4L2_PIX_FMT_SGBRG12:
 	case V4L2_PIX_FMT_SGRBG12: