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

Message ID 20201023174546.504028-5-paul.kocialkowski@bootlin.com
State New, archived
Headers show
Series
  • Allwinner MIPI CSI-2 support for A31/V3s/A83T
Related show

Commit Message

Paul Kocialkowski Oct. 23, 2020, 5:45 p.m. UTC
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(-)

Comments

Helen Koike Oct. 30, 2020, 10:45 p.m. UTC | #1
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?

Regards,
Helen
Paul Kocialkowski Nov. 4, 2020, 10:56 a.m. UTC | #2
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

Patch
diff mbox series

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;