Re: [PATCH 04/14] media: sun6i-csi: Fix the image storage bpp for 10/12-bit Bayer formats

Paul Kocialkowski
2020-11-04T10:56:29Z

[-- Attachment #1: Type: text/plain, Size: 2912 bytes --]

Hi Helen,

On Fri 30 Oct 20, 19:45, Helen Koike wrote:
> Hi Paul,
>
> On 10/23/20 2:45 PM, Paul Kocialkowski wrote:
> > Both 10 and 12-bit Bayer formats are stored aligned as 16-bit values
> > in memory, not unaligned 10 or 12 bits.
> >
> > Since the current code for retreiving the bpp is used only to
> > calculate the memory storage size of the picture (which is what
> > pixel formats describe, unlike media bus formats), fix it there.
> >
> > Fixes: 5cc7522d8965 ("media: sun6i: Add support for Allwinner CSI V3s")
> > Co-developed-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > Signed-off-by: Kévin L'hôpital <kevin.lhopital@bootlin.com>
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > ---
> > .../platform/sunxi/sun6i-csi/sun6i_csi.h | 20 +++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> > index c626821aaedb..7f2be70ae641 100644
> > --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> > +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.h
> > @@ -86,7 +86,7 @@ void sun6i_csi_update_buf_addr(struct sun6i_csi *csi, dma_addr_t addr);
> > */
> > void sun6i_csi_set_stream(struct sun6i_csi *csi, bool enable);
> >
> > -/* get bpp form v4l2 pixformat */
> > +/* get memory storage bpp from v4l2 pixformat */
> > static inline int sun6i_csi_get_bpp(unsigned int pixformat)
> > {
> > switch (pixformat) {
> > @@ -96,15 +96,6 @@ static inline int sun6i_csi_get_bpp(unsigned int pixformat)
> > case V4L2_PIX_FMT_SRGGB8:
> > case V4L2_PIX_FMT_JPEG:
> > return 8;
> > - case V4L2_PIX_FMT_SBGGR10:
> > - case V4L2_PIX_FMT_SGBRG10:
> > - case V4L2_PIX_FMT_SGRBG10:
> > - case V4L2_PIX_FMT_SRGGB10:
> > - return 10;
> > - case V4L2_PIX_FMT_SBGGR12:
> > - case V4L2_PIX_FMT_SGBRG12:
> > - case V4L2_PIX_FMT_SGRBG12:
> > - case V4L2_PIX_FMT_SRGGB12:
> > case V4L2_PIX_FMT_HM12:
> > case V4L2_PIX_FMT_NV12:
> > case V4L2_PIX_FMT_NV21:
> > @@ -121,6 +112,15 @@ static inline int sun6i_csi_get_bpp(unsigned int pixformat)
> > case V4L2_PIX_FMT_RGB565:
> > case V4L2_PIX_FMT_RGB565X:
> > return 16;
> > + case V4L2_PIX_FMT_SBGGR10:
> > + case V4L2_PIX_FMT_SGBRG10:
> > + case V4L2_PIX_FMT_SGRBG10:
> > + case V4L2_PIX_FMT_SRGGB10:
> > + case V4L2_PIX_FMT_SBGGR12:
> > + case V4L2_PIX_FMT_SGBRG12:
> > + case V4L2_PIX_FMT_SGRBG12:
> > + case V4L2_PIX_FMT_SRGGB12:
> > + return 16;
> > case V4L2_PIX_FMT_RGB24:
> > case V4L2_PIX_FMT_BGR24:
> > return 24;
> >
>
> Instead of updating this table, how about using v4l2_format_info() instead?

Yes that would be a very good thing to do indeed!

Thanks,

Paul

--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]