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

[Paul Kocialkowski]

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@xxxxxxxxxxx>
> > Signed-off-by: Kévin L'hôpital <kevin.lhopital@xxxxxxxxxxx>
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxx>
> > ---
> >  .../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: signature.asc
Description: PGP signature