[PATCH 0/6] YUV input support for the OMAP3 ISP

[PATCH 0/6] YUV input support for the OMAP3 ISP

[Laurent Pinchart]

Hi everybody,

Here are (long awaited) patches that implement support for YUV at the OMAP3 ISP
input.

Only progressive YUV in non-BT.656 mode is currently supported. The code has
been tested with an OV7725 sensor (I will push modifications to the ov772x
driver required to make it work with the OMAP3 ISP to mainline, but that
requires infrastructure work and will thus still take some time).

I've pushed the patches to the omap3isp-omap3isp-next branch of my git tree at
http://git.linuxtv.org/pinchartl/media.git, and rebased the
omap3isp-sensors-next and omap3isp-sensors-board branches.

My plan is to include this in the OMAP3 ISP v3.6 pull request (if no issue
found during review prevents this).

Ivaylo Petrov (1):
  omap3isp: csi2: Add V4L2_MBUS_FMT_YUYV8_2X8 support

Laurent Pinchart (5):
  omap3isp: video: Split format info bpp field into width and bpp
  omap3isp: video: Add YUYV8_2X8 and UYVY8_2X8 support
  omap3isp: ccdc: Remove support for interlaced data and master HS/VS
    mode
  omap3isp: ccdc: Remove ispccdc_syncif structure
  omap3isp: ccdc: Add YUV input formats support

 drivers/media/video/omap3isp/isp.c      |    4 +-
 drivers/media/video/omap3isp/isp.h      |    2 +-
 drivers/media/video/omap3isp/ispccdc.c  |  230 +++++++++++++++++++------------
 drivers/media/video/omap3isp/ispccdc.h  |   37 -----
 drivers/media/video/omap3isp/ispcsi2.c  |   27 ++++-
 drivers/media/video/omap3isp/ispvideo.c |   55 +++++---
 drivers/media/video/omap3isp/ispvideo.h |    6 +-
 include/media/omap3isp.h                |   14 +--
 8 files changed, 207 insertions(+), 168 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH 1/6] omap3isp: video: Split format info bpp field into width and bpp

[Laurent Pinchart]

The bpp field currently stores the sample width and is aligned to the
next multiple of 8 bits when computing data size in memory. This won't
work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
width and a bits per pixel value.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/ispccdc.c  |    8 +++---
 drivers/media/video/omap3isp/ispvideo.c |   45 ++++++++++++++++---------------
 drivers/media/video/omap3isp/ispvideo.h |    6 +++-
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index b74f7e9..e4231ef 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1143,12 +1143,12 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
 	if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
 		fmt_info = omap3isp_video_format_info(fmt_src.format.code);
-		depth_in = fmt_info->bpp;
+		depth_in = fmt_info->width;
 	}
 
 	fmt_info = omap3isp_video_format_info
 		(isp->isp_ccdc.formats[CCDC_PAD_SINK].code);
-	depth_out = fmt_info->bpp;
+	depth_out = fmt_info->width;
 
 	shift = depth_in - depth_out;
 	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift);
@@ -1179,7 +1179,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		syn_mode &= ~ISPCCDC_SYN_MODE_SDR2RSZ;
 
 	/* Use PACK8 mode for 1byte per pixel formats. */
-	if (omap3isp_video_format_info(format->code)->bpp <= 8)
+	if (omap3isp_video_format_info(format->code)->width <= 8)
 		syn_mode |= ISPCCDC_SYN_MODE_PACK8;
 	else
 		syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
@@ -2182,7 +2182,7 @@ static bool ccdc_is_shiftable(enum v4l2_mbus_pixelcode in,
 	if (in_info->flavor != out_info->flavor)
 		return false;
 
-	return in_info->bpp - out_info->bpp + additional_shift <= 6;
+	return in_info->width - out_info->width + additional_shift <= 6;
 }
 
 static int ccdc_link_validate(struct v4l2_subdev *sd,
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index b37379d..f0ed2ec 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -53,67 +53,67 @@
 static struct isp_format_info formats[] = {
 	{ V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
 	  V4L2_MBUS_FMT_Y8_1X8, V4L2_MBUS_FMT_Y8_1X8,
-	  V4L2_PIX_FMT_GREY, 8, },
+	  V4L2_PIX_FMT_GREY, 8, 8, },
 	{ V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y10_1X10,
 	  V4L2_MBUS_FMT_Y10_1X10, V4L2_MBUS_FMT_Y8_1X8,
-	  V4L2_PIX_FMT_Y10, 10, },
+	  V4L2_PIX_FMT_Y10, 10, 16, },
 	{ V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y10_1X10,
 	  V4L2_MBUS_FMT_Y12_1X12, V4L2_MBUS_FMT_Y8_1X8,
-	  V4L2_PIX_FMT_Y12, 12, },
+	  V4L2_PIX_FMT_Y12, 12, 16, },
 	{ V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
 	  V4L2_MBUS_FMT_SBGGR8_1X8, V4L2_MBUS_FMT_SBGGR8_1X8,
-	  V4L2_PIX_FMT_SBGGR8, 8, },
+	  V4L2_PIX_FMT_SBGGR8, 8, 8, },
 	{ V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
 	  V4L2_MBUS_FMT_SGBRG8_1X8, V4L2_MBUS_FMT_SGBRG8_1X8,
-	  V4L2_PIX_FMT_SGBRG8, 8, },
+	  V4L2_PIX_FMT_SGBRG8, 8, 8, },
 	{ V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
 	  V4L2_MBUS_FMT_SGRBG8_1X8, V4L2_MBUS_FMT_SGRBG8_1X8,
-	  V4L2_PIX_FMT_SGRBG8, 8, },
+	  V4L2_PIX_FMT_SGRBG8, 8, 8, },
 	{ V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
 	  V4L2_MBUS_FMT_SRGGB8_1X8, V4L2_MBUS_FMT_SRGGB8_1X8,
-	  V4L2_PIX_FMT_SRGGB8, 8, },
+	  V4L2_PIX_FMT_SRGGB8, 8, 8, },
 	{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8,
 	  V4L2_MBUS_FMT_SBGGR10_1X10, 0,
-	  V4L2_PIX_FMT_SBGGR10DPCM8, 8, },
+	  V4L2_PIX_FMT_SBGGR10DPCM8, 8, 8, },
 	{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8,
 	  V4L2_MBUS_FMT_SGBRG10_1X10, 0,
-	  V4L2_PIX_FMT_SGBRG10DPCM8, 8, },
+	  V4L2_PIX_FMT_SGBRG10DPCM8, 8, 8, },
 	{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8,
 	  V4L2_MBUS_FMT_SGRBG10_1X10, 0,
-	  V4L2_PIX_FMT_SGRBG10DPCM8, 8, },
+	  V4L2_PIX_FMT_SGRBG10DPCM8, 8, 8, },
 	{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8,
 	  V4L2_MBUS_FMT_SRGGB10_1X10, 0,
-	  V4L2_PIX_FMT_SRGGB10DPCM8, 8, },
+	  V4L2_PIX_FMT_SRGGB10DPCM8, 8, 8, },
 	{ V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR10_1X10,
 	  V4L2_MBUS_FMT_SBGGR10_1X10, V4L2_MBUS_FMT_SBGGR8_1X8,
-	  V4L2_PIX_FMT_SBGGR10, 10, },
+	  V4L2_PIX_FMT_SBGGR10, 10, 16, },
 	{ V4L2_MBUS_FMT_SGBRG10_1X10, V4L2_MBUS_FMT_SGBRG10_1X10,
 	  V4L2_MBUS_FMT_SGBRG10_1X10, V4L2_MBUS_FMT_SGBRG8_1X8,
-	  V4L2_PIX_FMT_SGBRG10, 10, },
+	  V4L2_PIX_FMT_SGBRG10, 10, 16, },
 	{ V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SGRBG10_1X10,
 	  V4L2_MBUS_FMT_SGRBG10_1X10, V4L2_MBUS_FMT_SGRBG8_1X8,
-	  V4L2_PIX_FMT_SGRBG10, 10, },
+	  V4L2_PIX_FMT_SGRBG10, 10, 16, },
 	{ V4L2_MBUS_FMT_SRGGB10_1X10, V4L2_MBUS_FMT_SRGGB10_1X10,
 	  V4L2_MBUS_FMT_SRGGB10_1X10, V4L2_MBUS_FMT_SRGGB8_1X8,
-	  V4L2_PIX_FMT_SRGGB10, 10, },
+	  V4L2_PIX_FMT_SRGGB10, 10, 16, },
 	{ V4L2_MBUS_FMT_SBGGR12_1X12, V4L2_MBUS_FMT_SBGGR10_1X10,
 	  V4L2_MBUS_FMT_SBGGR12_1X12, V4L2_MBUS_FMT_SBGGR8_1X8,
-	  V4L2_PIX_FMT_SBGGR12, 12, },
+	  V4L2_PIX_FMT_SBGGR12, 12, 16, },
 	{ V4L2_MBUS_FMT_SGBRG12_1X12, V4L2_MBUS_FMT_SGBRG10_1X10,
 	  V4L2_MBUS_FMT_SGBRG12_1X12, V4L2_MBUS_FMT_SGBRG8_1X8,
-	  V4L2_PIX_FMT_SGBRG12, 12, },
+	  V4L2_PIX_FMT_SGBRG12, 12, 16, },
 	{ V4L2_MBUS_FMT_SGRBG12_1X12, V4L2_MBUS_FMT_SGRBG10_1X10,
 	  V4L2_MBUS_FMT_SGRBG12_1X12, V4L2_MBUS_FMT_SGRBG8_1X8,
-	  V4L2_PIX_FMT_SGRBG12, 12, },
+	  V4L2_PIX_FMT_SGRBG12, 12, 16, },
 	{ V4L2_MBUS_FMT_SRGGB12_1X12, V4L2_MBUS_FMT_SRGGB10_1X10,
 	  V4L2_MBUS_FMT_SRGGB12_1X12, V4L2_MBUS_FMT_SRGGB8_1X8,
-	  V4L2_PIX_FMT_SRGGB12, 12, },
+	  V4L2_PIX_FMT_SRGGB12, 12, 16, },
 	{ V4L2_MBUS_FMT_UYVY8_1X16, V4L2_MBUS_FMT_UYVY8_1X16,
 	  V4L2_MBUS_FMT_UYVY8_1X16, 0,
-	  V4L2_PIX_FMT_UYVY, 16, },
+	  V4L2_PIX_FMT_UYVY, 16, 16, },
 	{ V4L2_MBUS_FMT_YUYV8_1X16, V4L2_MBUS_FMT_YUYV8_1X16,
 	  V4L2_MBUS_FMT_YUYV8_1X16, 0,
-	  V4L2_PIX_FMT_YUYV, 16, },
+	  V4L2_PIX_FMT_YUYV, 16, 16, },
 };
 
 const struct isp_format_info *
@@ -921,7 +921,8 @@ static int isp_video_check_external_subdevs(struct isp_video *video,
 		return ret;
 	}
 
-	pipe->external_bpp = omap3isp_video_format_info(fmt.format.code)->bpp;
+	pipe->external_width =
+		omap3isp_video_format_info(fmt.format.code)->width;
 
 	memset(&ctrls, 0, sizeof(ctrls));
 	memset(&ctrl, 0, sizeof(ctrl));
diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
index 5acc909..f8092cc 100644
--- a/drivers/media/video/omap3isp/ispvideo.h
+++ b/drivers/media/video/omap3isp/ispvideo.h
@@ -51,7 +51,8 @@ struct v4l2_pix_format;
  * @flavor: V4L2 media bus format code for the same pixel layout but
  *	shifted to be 8 bits per pixel. =0 if format is not shiftable.
  * @pixelformat: V4L2 pixel format FCC identifier
- * @bpp: Bits per pixel
+ * @width: Data bus width
+ * @bpp: Bits per pixel (when stored in memory)
  */
 struct isp_format_info {
 	enum v4l2_mbus_pixelcode code;
@@ -59,6 +60,7 @@ struct isp_format_info {
 	enum v4l2_mbus_pixelcode uncompressed;
 	enum v4l2_mbus_pixelcode flavor;
 	u32 pixelformat;
+	unsigned int width;
 	unsigned int bpp;
 };
 
@@ -106,7 +108,7 @@ struct isp_pipeline {
 	struct v4l2_fract max_timeperframe;
 	struct v4l2_subdev *external;
 	unsigned int external_rate;
-	unsigned int external_bpp;
+	unsigned int external_width;
 };
 
 #define to_isp_pipeline(__e) \
-- 
1.7.3.4

Re: [PATCH 1/6] omap3isp: video: Split format info bpp field into width and bpp

[Sakari Ailus]

Hi Laurent,

Thanks for the patch.

Laurent Pinchart wrote:
> The bpp field currently stores the sample width and is aligned to the
> next multiple of 8 bits when computing data size in memory. This won't
> work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
> width and a bits per pixel value.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

...

> diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
> index 5acc909..f8092cc 100644
> --- a/drivers/media/video/omap3isp/ispvideo.h
> +++ b/drivers/media/video/omap3isp/ispvideo.h
> @@ -51,7 +51,8 @@ struct v4l2_pix_format;
>   * @flavor: V4L2 media bus format code for the same pixel layout but
>   *	shifted to be 8 bits per pixel. =0 if format is not shiftable.
>   * @pixelformat: V4L2 pixel format FCC identifier
> - * @bpp: Bits per pixel
> + * @width: Data bus width
> + * @bpp: Bits per pixel (when stored in memory)

Would it make sense to use bytes rather than bits?

Also width isn't really the width of the data bus on serial busses, is
it? How about busses that transfer pixels 8 bits at the time?

You can also stop using ALIGN() in isp_video_mbus_to_pix() (in
ispvideo.c) as the ISP will always write complete bytes.

I might even switch the meaning between width and bpp but this is up to you.

>   */
>  struct isp_format_info {
>  	enum v4l2_mbus_pixelcode code;
> @@ -59,6 +60,7 @@ struct isp_format_info {
>  	enum v4l2_mbus_pixelcode uncompressed;
>  	enum v4l2_mbus_pixelcode flavor;
>  	u32 pixelformat;
> +	unsigned int width;
>  	unsigned int bpp;
>  };
>  
> @@ -106,7 +108,7 @@ struct isp_pipeline {
>  	struct v4l2_fract max_timeperframe;
>  	struct v4l2_subdev *external;
>  	unsigned int external_rate;
> -	unsigned int external_bpp;
> +	unsigned int external_width;
>  };
>  
>  #define to_isp_pipeline(__e) \
> 

Cheers,

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [PATCH 1/6] omap3isp: video: Split format info bpp field into width and bpp

[Laurent Pinchart]

Hi Sakari,

Thanks for the review.

On Wednesday 27 June 2012 14:07:51 Sakari Ailus wrote:
> Laurent Pinchart wrote:
> > The bpp field currently stores the sample width and is aligned to the
> > next multiple of 8 bits when computing data size in memory. This won't
> > work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
> > width and a bits per pixel value.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> ...
> 
> > diff --git a/drivers/media/video/omap3isp/ispvideo.h
> > b/drivers/media/video/omap3isp/ispvideo.h index 5acc909..f8092cc 100644
> > --- a/drivers/media/video/omap3isp/ispvideo.h
> > +++ b/drivers/media/video/omap3isp/ispvideo.h
> > @@ -51,7 +51,8 @@ struct v4l2_pix_format;
> > 
> >   * @flavor: V4L2 media bus format code for the same pixel layout but
> >   *	shifted to be 8 bits per pixel. =0 if format is not shiftable.
> >   * @pixelformat: V4L2 pixel format FCC identifier
> > 
> > - * @bpp: Bits per pixel
> > + * @width: Data bus width
> > + * @bpp: Bits per pixel (when stored in memory)
> 
> Would it make sense to use bytes rather than bits?

I'll change that.

> Also width isn't really the width of the data bus on serial busses, is it?
> How about busses that transfer pixels 8 bits at the time?

I could change the comment to "bits per pixel (when transferred on a bus)", 
would that be better ?

> You can also stop using ALIGN() in isp_video_mbus_to_pix() (in ispvideo.c)
> as the ISP will always write complete bytes.

Indeed.

> I might even switch the meaning between width and bpp but this is up to you.

I got used to width/bpp as defined by this patch, so that would be confusing 
:-)

> >   */
> >  
> >  struct isp_format_info {
> >  
> >  	enum v4l2_mbus_pixelcode code;
> > 
> > @@ -59,6 +60,7 @@ struct isp_format_info {
> > 
> >  	enum v4l2_mbus_pixelcode uncompressed;
> >  	enum v4l2_mbus_pixelcode flavor;
> >  	u32 pixelformat;
> > 
> > +	unsigned int width;
> > 
> >  	unsigned int bpp;
> >  
> >  };
> > 
> > @@ -106,7 +108,7 @@ struct isp_pipeline {
> > 
> >  	struct v4l2_fract max_timeperframe;
> >  	struct v4l2_subdev *external;
> >  	unsigned int external_rate;
> > 
> > -	unsigned int external_bpp;
> > +	unsigned int external_width;
> > 
> >  };
> >  
> >  #define to_isp_pipeline(__e) \

-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] omap3isp: video: Split format info bpp field into width and bpp

[Laurent Pinchart]

On Wednesday 27 June 2012 13:54:30 Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On Wednesday 27 June 2012 14:07:51 Sakari Ailus wrote:
> > Laurent Pinchart wrote:
> > > The bpp field currently stores the sample width and is aligned to the
> > > next multiple of 8 bits when computing data size in memory. This won't
> > > work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
> > > width and a bits per pixel value.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > ...
> > 
> > > diff --git a/drivers/media/video/omap3isp/ispvideo.h
> > > b/drivers/media/video/omap3isp/ispvideo.h index 5acc909..f8092cc 100644
> > > --- a/drivers/media/video/omap3isp/ispvideo.h
> > > +++ b/drivers/media/video/omap3isp/ispvideo.h
> > > @@ -51,7 +51,8 @@ struct v4l2_pix_format;
> > > 
> > >   * @flavor: V4L2 media bus format code for the same pixel layout but
> > >   *	shifted to be 8 bits per pixel. =0 if format is not shiftable.
> > >   * @pixelformat: V4L2 pixel format FCC identifier
> > > 
> > > - * @bpp: Bits per pixel
> > > + * @width: Data bus width
> > > + * @bpp: Bits per pixel (when stored in memory)
> > 
> > Would it make sense to use bytes rather than bits?
> 
> I'll change that.
> 
> > Also width isn't really the width of the data bus on serial busses, is it?
> > How about busses that transfer pixels 8 bits at the time?
> 
> I could change the comment to "bits per pixel (when transferred on a bus)",
> would that be better ?

s/pixel/sample/

> > You can also stop using ALIGN() in isp_video_mbus_to_pix() (in ispvideo.c)
> > as the ISP will always write complete bytes.
> 
> Indeed.
> 
> > I might even switch the meaning between width and bpp but this is up to
> > you.
> I got used to width/bpp as defined by this patch, so that would be confusing
> :-)
> :
> > >   */
> > >  
> > >  struct isp_format_info {
> > >  
> > >  	enum v4l2_mbus_pixelcode code;
> > > 
> > > @@ -59,6 +60,7 @@ struct isp_format_info {
> > > 
> > >  	enum v4l2_mbus_pixelcode uncompressed;
> > >  	enum v4l2_mbus_pixelcode flavor;
> > >  	u32 pixelformat;
> > > 
> > > +	unsigned int width;
> > > 
> > >  	unsigned int bpp;
> > >  
> > >  };
> > > 
> > > @@ -106,7 +108,7 @@ struct isp_pipeline {
> > > 
> > >  	struct v4l2_fract max_timeperframe;
> > >  	struct v4l2_subdev *external;
> > >  	unsigned int external_rate;
> > > 
> > > -	unsigned int external_bpp;
> > > +	unsigned int external_width;
> > > 
> > >  };
> > >  
> > >  #define to_isp_pipeline(__e) \
-- 
Regards,

Laurent Pinchart

Re: [PATCH 1/6] omap3isp: video: Split format info bpp field into width and bpp

[Sakari Ailus]

Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thanks for the review.
> 
> On Wednesday 27 June 2012 14:07:51 Sakari Ailus wrote:
>> Laurent Pinchart wrote:
>>> The bpp field currently stores the sample width and is aligned to the
>>> next multiple of 8 bits when computing data size in memory. This won't
>>> work anymore for YUYV8_2X8 formats. Split the bpp field into a sample
>>> width and a bits per pixel value.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>
>> ...
>>
>>> diff --git a/drivers/media/video/omap3isp/ispvideo.h
>>> b/drivers/media/video/omap3isp/ispvideo.h index 5acc909..f8092cc 100644
>>> --- a/drivers/media/video/omap3isp/ispvideo.h
>>> +++ b/drivers/media/video/omap3isp/ispvideo.h
>>> @@ -51,7 +51,8 @@ struct v4l2_pix_format;
>>>
>>>   * @flavor: V4L2 media bus format code for the same pixel layout but
>>>   *	shifted to be 8 bits per pixel. =0 if format is not shiftable.
>>>   * @pixelformat: V4L2 pixel format FCC identifier
>>>
>>> - * @bpp: Bits per pixel
>>> + * @width: Data bus width
>>> + * @bpp: Bits per pixel (when stored in memory)
>>
>> Would it make sense to use bytes rather than bits?
> 
> I'll change that.
> 
>> Also width isn't really the width of the data bus on serial busses, is it?
>> How about busses that transfer pixels 8 bits at the time?
> 
> I could change the comment to "bits per pixel (when transferred on a bus)", 
> would that be better ?

Pixels (or samples, as you suggested later) sounds good to me.

>> You can also stop using ALIGN() in isp_video_mbus_to_pix() (in ispvideo.c)
>> as the ISP will always write complete bytes.
> 
> Indeed.

With these changes,

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

-- 
Sakari Ailus
sakari.ailus@iki.fi

[PATCH 2/6] omap3isp: video: Add YUYV8_2X8 and UYVY8_2X8 support

[Laurent Pinchart]

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/ispvideo.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index f0ed2ec..9ce158e 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -114,6 +114,12 @@ static struct isp_format_info formats[] = {
 	{ V4L2_MBUS_FMT_YUYV8_1X16, V4L2_MBUS_FMT_YUYV8_1X16,
 	  V4L2_MBUS_FMT_YUYV8_1X16, 0,
 	  V4L2_PIX_FMT_YUYV, 16, 16, },
+	{ V4L2_MBUS_FMT_UYVY8_2X8, V4L2_MBUS_FMT_UYVY8_2X8,
+	  V4L2_MBUS_FMT_UYVY8_2X8, 0,
+	  V4L2_PIX_FMT_UYVY, 8, 16, },
+	{ V4L2_MBUS_FMT_YUYV8_2X8, V4L2_MBUS_FMT_YUYV8_2X8,
+	  V4L2_MBUS_FMT_YUYV8_2X8, 0,
+	  V4L2_PIX_FMT_YUYV, 8, 16, },
 };
 
 const struct isp_format_info *
-- 
1.7.3.4

[PATCH 3/6] omap3isp: csi2: Add V4L2_MBUS_FMT_YUYV8_2X8 support

[Laurent Pinchart]

From: Ivaylo Petrov <ivpetrov@mm-sol.com>

Tested with ov9740 and

struct isp_csi2_platform_data {
	.interface = ISP_INTERFACE_CSI2A_PHY2,
	.bus = {
		.csi2 = {
			.crc		= 1,
			.vpclk_div	= 1,
		}
	},
}

Signed-off-by: Ivaylo Petrov <ivpetrov@mm-sol.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/ispcsi2.c |   27 +++++++++++++++++++++++++--
 1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispcsi2.c b/drivers/media/video/omap3isp/ispcsi2.c
index a172436..6a3ff79 100644
--- a/drivers/media/video/omap3isp/ispcsi2.c
+++ b/drivers/media/video/omap3isp/ispcsi2.c
@@ -96,11 +96,12 @@ static const unsigned int csi2_input_fmts[] = {
 	V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8,
 	V4L2_MBUS_FMT_SGBRG10_1X10,
 	V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8,
+	V4L2_MBUS_FMT_YUYV8_2X8,
 };
 
 /* To set the format on the CSI2 requires a mapping function that takes
  * the following inputs:
- * - 2 different formats (at this time)
+ * - 3 different formats (at this time)
  * - 2 destinations (mem, vp+mem) (vp only handled separately)
  * - 2 decompression options (on, off)
  * - 2 isp revisions (certain format must be handled differently on OMAP3630)
@@ -108,7 +109,7 @@ static const unsigned int csi2_input_fmts[] = {
  * Array indices as follows: [format][dest][decompr][is_3630]
  * Not all combinations are valid. 0 means invalid.
  */
-static const u16 __csi2_fmt_map[2][2][2][2] = {
+static const u16 __csi2_fmt_map[3][2][2][2] = {
 	/* RAW10 formats */
 	{
 		/* Output to memory */
@@ -147,6 +148,25 @@ static const u16 __csi2_fmt_map[2][2][2][2] = {
 			  CSI2_USERDEF_8BIT_DATA1_DPCM10_VP },
 		},
 	},
+	/* YUYV8 2X8 formats */
+	{
+		/* Output to memory */
+		{
+			/* No DPCM decompression */
+			{ CSI2_PIX_FMT_YUV422_8BIT,
+			  CSI2_PIX_FMT_YUV422_8BIT },
+			/* DPCM decompression */
+			{ 0, 0 },
+		},
+		/* Output to both */
+		{
+			/* No DPCM decompression */
+			{ CSI2_PIX_FMT_YUV422_8BIT_VP,
+			  CSI2_PIX_FMT_YUV422_8BIT_VP },
+			/* DPCM decompression */
+			{ 0, 0 },
+		},
+	},
 };
 
 /*
@@ -173,6 +193,9 @@ static u16 csi2_ctx_map_format(struct isp_csi2_device *csi2)
 	case V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8:
 		fmtidx = 1;
 		break;
+	case V4L2_MBUS_FMT_YUYV8_2X8:
+		fmtidx = 2;
+		break;
 	default:
 		WARN(1, KERN_ERR "CSI2: pixel format %08x unsupported!\n",
 		     fmt->code);
-- 
1.7.3.4

[PATCH 4/6] omap3isp: ccdc: Remove support for interlaced data and master HS/VS mode

[Laurent Pinchart]

Those features are half-implemented and not used. Remove them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/ispccdc.c |   51 +------------------------------
 drivers/media/video/omap3isp/ispccdc.h |   18 -----------
 2 files changed, 2 insertions(+), 67 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index e4231ef..bdd7d72 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -967,17 +967,8 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 				struct ispccdc_syncif *syncif)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
-	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
-				     ISPCCDC_SYN_MODE);
+	u32 syn_mode = ISPCCDC_SYN_MODE_VDHDEN;
 
-	syn_mode |= ISPCCDC_SYN_MODE_VDHDEN;
-
-	if (syncif->fldstat)
-		syn_mode |= ISPCCDC_SYN_MODE_FLDSTAT;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_FLDSTAT;
-
-	syn_mode &= ~ISPCCDC_SYN_MODE_DATSIZ_MASK;
 	switch (syncif->datsz) {
 	case 8:
 		syn_mode |= ISPCCDC_SYN_MODE_DATSIZ_8;
@@ -993,47 +984,14 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 		break;
 	}
 
-	if (syncif->fldmode)
-		syn_mode |= ISPCCDC_SYN_MODE_FLDMODE;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_FLDMODE;
-
 	if (syncif->datapol)
 		syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_DATAPOL;
-
-	if (syncif->fldpol)
-		syn_mode |= ISPCCDC_SYN_MODE_FLDPOL;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_FLDPOL;
 
 	if (syncif->hdpol)
 		syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_HDPOL;
 
 	if (syncif->vdpol)
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_VDPOL;
-
-	if (syncif->ccdc_mastermode) {
-		syn_mode |= ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT;
-		isp_reg_writel(isp,
-			       syncif->hs_width << ISPCCDC_HD_VD_WID_HDW_SHIFT
-			     | syncif->vs_width << ISPCCDC_HD_VD_WID_VDW_SHIFT,
-			       OMAP3_ISP_IOMEM_CCDC,
-			       ISPCCDC_HD_VD_WID);
-
-		isp_reg_writel(isp,
-			       syncif->ppln << ISPCCDC_PIX_LINES_PPLN_SHIFT
-			     | syncif->hlprf << ISPCCDC_PIX_LINES_HLPRF_SHIFT,
-			       OMAP3_ISP_IOMEM_CCDC,
-			       ISPCCDC_PIX_LINES);
-	} else
-		syn_mode &= ~(ISPCCDC_SYN_MODE_FLDOUT |
-			      ISPCCDC_SYN_MODE_VDHDOUT);
 
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
@@ -1154,6 +1112,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift);
 
 	ccdc->syncif.datsz = depth_out;
+	ccdc->syncif.datapol = 0;
 	ccdc->syncif.hdpol = pdata ? pdata->hs_pol : 0;
 	ccdc->syncif.vdpol = pdata ? pdata->vs_pol : 0;
 	ccdc_config_sync_if(ccdc, &ccdc->syncif);
@@ -2487,13 +2446,7 @@ int omap3isp_ccdc_init(struct isp_device *isp)
 	INIT_LIST_HEAD(&ccdc->lsc.free_queue);
 	spin_lock_init(&ccdc->lsc.req_lock);
 
-	ccdc->syncif.ccdc_mastermode = 0;
-	ccdc->syncif.datapol = 0;
 	ccdc->syncif.datsz = 0;
-	ccdc->syncif.fldmode = 0;
-	ccdc->syncif.fldout = 0;
-	ccdc->syncif.fldpol = 0;
-	ccdc->syncif.fldstat = 0;
 
 	ccdc->clamp.oblen = 0;
 	ccdc->clamp.dcsubval = 0;
diff --git a/drivers/media/video/omap3isp/ispccdc.h b/drivers/media/video/omap3isp/ispccdc.h
index 890f6b3..57007d7 100644
--- a/drivers/media/video/omap3isp/ispccdc.h
+++ b/drivers/media/video/omap3isp/ispccdc.h
@@ -48,35 +48,17 @@ enum ccdc_input_entity {
 
 /*
  * struct ispccdc_syncif - Structure for Sync Interface between sensor and CCDC
- * @ccdc_mastermode: Master mode. 1 - Master, 0 - Slave.
- * @fldstat: Field state. 0 - Odd Field, 1 - Even Field.
  * @datsz: Data size.
- * @fldmode: 0 - Progressive, 1 - Interlaced.
  * @datapol: 0 - Positive, 1 - Negative.
- * @fldpol: 0 - Positive, 1 - Negative.
  * @hdpol: 0 - Positive, 1 - Negative.
  * @vdpol: 0 - Positive, 1 - Negative.
- * @fldout: 0 - Input, 1 - Output.
- * @hs_width: Width of the Horizontal Sync pulse, used for HS/VS Output.
- * @vs_width: Width of the Vertical Sync pulse, used for HS/VS Output.
- * @ppln: Number of pixels per line, used for HS/VS Output.
- * @hlprf: Number of half lines per frame, used for HS/VS Output.
  * @bt_r656_en: 1 - Enable ITU-R BT656 mode, 0 - Sync mode.
  */
 struct ispccdc_syncif {
-	u8 ccdc_mastermode;
-	u8 fldstat;
 	u8 datsz;
-	u8 fldmode;
 	u8 datapol;
-	u8 fldpol;
 	u8 hdpol;
 	u8 vdpol;
-	u8 fldout;
-	u8 hs_width;
-	u8 vs_width;
-	u8 ppln;
-	u8 hlprf;
 	u8 bt_r656_en;
 };
 
-- 
1.7.3.4

Re: [PATCH 4/6] omap3isp: ccdc: Remove support for interlaced data and master HS/VS mode

[Sakari Ailus]

Laurent Pinchart wrote:
> Those features are half-implemented and not used. Remove them.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!!!!!

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

-- 
Sakari Ailus
sakari.ailus@iki.fi

[PATCH 5/6] omap3isp: ccdc: Remove ispccdc_syncif structure

[Laurent Pinchart]

The structure is only used to store configuration data and pass it to
CCDC configuration functions. Access the data directly from the
locations that need it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/ispccdc.c |   38 ++++++++++++++-----------------
 drivers/media/video/omap3isp/ispccdc.h |   19 ----------------
 include/media/omap3isp.h               |    3 ++
 3 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index bdd7d72..d5aed79 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -627,9 +627,12 @@ static void ccdc_configure_lpf(struct isp_ccdc_device *ccdc)
 static void ccdc_configure_alaw(struct isp_ccdc_device *ccdc)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
+	const struct isp_format_info *info;
 	u32 alaw = 0;
 
-	switch (ccdc->syncif.datsz) {
+	info = omap3isp_video_format_info(ccdc->formats[CCDC_PAD_SINK].code);
+
+	switch (info->width) {
 	case 8:
 		return;
 
@@ -813,6 +816,7 @@ static void ccdc_config_vp(struct isp_ccdc_device *ccdc)
 {
 	struct isp_pipeline *pipe = to_isp_pipeline(&ccdc->subdev.entity);
 	struct isp_device *isp = to_isp_device(ccdc);
+	const struct isp_format_info *info;
 	unsigned long l3_ick = pipe->l3_ick;
 	unsigned int max_div = isp->revision == ISP_REVISION_15_0 ? 64 : 8;
 	unsigned int div = 0;
@@ -821,7 +825,9 @@ static void ccdc_config_vp(struct isp_ccdc_device *ccdc)
 	fmtcfg_vp = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_FMTCFG)
 		  & ~(ISPCCDC_FMTCFG_VPIN_MASK | ISPCCDC_FMTCFG_VPIF_FRQ_MASK);
 
-	switch (ccdc->syncif.datsz) {
+	info = omap3isp_video_format_info(ccdc->formats[CCDC_PAD_SINK].code);
+
+	switch (info->width) {
 	case 8:
 	case 10:
 		fmtcfg_vp |= ISPCCDC_FMTCFG_VPIN_9_0;
@@ -959,17 +965,17 @@ void omap3isp_ccdc_max_rate(struct isp_ccdc_device *ccdc,
 /*
  * ccdc_config_sync_if - Set CCDC sync interface configuration
  * @ccdc: Pointer to ISP CCDC device.
- * @syncif: Structure containing the sync parameters like field state, CCDC in
- *          master/slave mode, raw/yuv data, polarity of data, field, hs, vs
- *          signals.
+ * @pdata: Parallel interface platform data (may be NULL)
+ * @data_size: Data size
  */
 static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
-				struct ispccdc_syncif *syncif)
+				struct isp_parallel_platform_data *pdata,
+				unsigned int data_size)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
 	u32 syn_mode = ISPCCDC_SYN_MODE_VDHDEN;
 
-	switch (syncif->datsz) {
+	switch (data_size) {
 	case 8:
 		syn_mode |= ISPCCDC_SYN_MODE_DATSIZ_8;
 		break;
@@ -984,20 +990,16 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 		break;
 	}
 
-	if (syncif->datapol)
+	if (pdata && pdata->data_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
 
-	if (syncif->hdpol)
+	if (pdata && pdata->hs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
 
-	if (syncif->vdpol)
+	if (pdata && pdata->vs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
 
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
-
-	if (!syncif->bt_r656_en)
-		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
 }
 
 /* CCDC formats descriptions */
@@ -1111,11 +1113,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	shift = depth_in - depth_out;
 	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift);
 
-	ccdc->syncif.datsz = depth_out;
-	ccdc->syncif.datapol = 0;
-	ccdc->syncif.hdpol = pdata ? pdata->hs_pol : 0;
-	ccdc->syncif.vdpol = pdata ? pdata->vs_pol : 0;
-	ccdc_config_sync_if(ccdc, &ccdc->syncif);
+	ccdc_config_sync_if(ccdc, pdata, depth_out);
 
 	/* CCDC_PAD_SINK */
 	format = &ccdc->formats[CCDC_PAD_SINK];
@@ -2446,8 +2444,6 @@ int omap3isp_ccdc_init(struct isp_device *isp)
 	INIT_LIST_HEAD(&ccdc->lsc.free_queue);
 	spin_lock_init(&ccdc->lsc.req_lock);
 
-	ccdc->syncif.datsz = 0;
-
 	ccdc->clamp.oblen = 0;
 	ccdc->clamp.dcsubval = 0;
 
diff --git a/drivers/media/video/omap3isp/ispccdc.h b/drivers/media/video/omap3isp/ispccdc.h
index 57007d7..a5da9e1 100644
--- a/drivers/media/video/omap3isp/ispccdc.h
+++ b/drivers/media/video/omap3isp/ispccdc.h
@@ -46,22 +46,6 @@ enum ccdc_input_entity {
 
 #define	OMAP3ISP_CCDC_NEVENTS	16
 
-/*
- * struct ispccdc_syncif - Structure for Sync Interface between sensor and CCDC
- * @datsz: Data size.
- * @datapol: 0 - Positive, 1 - Negative.
- * @hdpol: 0 - Positive, 1 - Negative.
- * @vdpol: 0 - Positive, 1 - Negative.
- * @bt_r656_en: 1 - Enable ITU-R BT656 mode, 0 - Sync mode.
- */
-struct ispccdc_syncif {
-	u8 datsz;
-	u8 datapol;
-	u8 hdpol;
-	u8 vdpol;
-	u8 bt_r656_en;
-};
-
 enum ispccdc_lsc_state {
 	LSC_STATE_STOPPED = 0,
 	LSC_STATE_STOPPING = 1,
@@ -135,7 +119,6 @@ struct ispccdc_lsc {
  * @lsc: Lens shading compensation configuration
  * @update: Bitmask of controls to update during the next interrupt
  * @shadow_update: Controls update in progress by userspace
- * @syncif: Interface synchronization configuration
  * @underrun: A buffer underrun occurred and a new buffer has been queued
  * @state: Streaming state
  * @lock: Serializes shadow_update with interrupt handler
@@ -164,8 +147,6 @@ struct isp_ccdc_device {
 	unsigned int update;
 	unsigned int shadow_update;
 
-	struct ispccdc_syncif syncif;
-
 	unsigned int underrun:1;
 	enum isp_pipeline_stream_state state;
 	spinlock_t lock;
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 4d94be5..5ab9449 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -67,6 +67,8 @@ enum {
  *		0 - Active high, 1 - Active low
  * @vs_pol: Vertical synchronization polarity
  *		0 - Active high, 1 - Active low
+ * @data_pol: Data polarity
+ *		0 - Normal, 1 - One's complement
  * @bridge: CCDC Bridge input control
  *		ISP_BRIDGE_DISABLE - Disable
  *		ISP_BRIDGE_LITTLE_ENDIAN - Little endian
@@ -77,6 +79,7 @@ struct isp_parallel_platform_data {
 	unsigned int clk_pol:1;
 	unsigned int hs_pol:1;
 	unsigned int vs_pol:1;
+	unsigned int data_pol:1;
 	unsigned int bridge:2;
 };
 
-- 
1.7.3.4

Re: [PATCH 5/6] omap3isp: ccdc: Remove ispccdc_syncif structure

[Sakari Ailus]

Laurent Pinchart wrote:
> The structure is only used to store configuration data and pass it to
> CCDC configuration functions. Access the data directly from the
> locations that need it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!!

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>


-- 
Sakari Ailus
sakari.ailus@iki.fi

[PATCH 6/6] omap3isp: ccdc: Add YUV input formats support

[Laurent Pinchart]

Enable the bridge automatically when the input format is YUYV8 or UYVY8.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/video/omap3isp/isp.c      |    4 +-
 drivers/media/video/omap3isp/isp.h      |    2 +-
 drivers/media/video/omap3isp/ispccdc.c  |  145 ++++++++++++++++++++++++++-----
 drivers/media/video/omap3isp/ispvideo.c |    4 +
 include/media/omap3isp.h                |   11 ---
 5 files changed, 129 insertions(+), 37 deletions(-)

diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c
index 36805ca..e0096e0 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -293,7 +293,7 @@ static void isp_core_init(struct isp_device *isp, int idle)
 void omap3isp_configure_bridge(struct isp_device *isp,
 			       enum ccdc_input_entity input,
 			       const struct isp_parallel_platform_data *pdata,
-			       unsigned int shift)
+			       unsigned int shift, unsigned int bridge)
 {
 	u32 ispctrl_val;
 
@@ -302,12 +302,12 @@ void omap3isp_configure_bridge(struct isp_device *isp,
 	ispctrl_val &= ~ISPCTRL_PAR_CLK_POL_INV;
 	ispctrl_val &= ~ISPCTRL_PAR_SER_CLK_SEL_MASK;
 	ispctrl_val &= ~ISPCTRL_PAR_BRIDGE_MASK;
+	ispctrl_val |= bridge;
 
 	switch (input) {
 	case CCDC_INPUT_PARALLEL:
 		ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
 		ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
-		ispctrl_val |= pdata->bridge << ISPCTRL_PAR_BRIDGE_SHIFT;
 		shift += pdata->data_lane_shift * 2;
 		break;
 
diff --git a/drivers/media/video/omap3isp/isp.h b/drivers/media/video/omap3isp/isp.h
index ba2159b..8be7487 100644
--- a/drivers/media/video/omap3isp/isp.h
+++ b/drivers/media/video/omap3isp/isp.h
@@ -236,7 +236,7 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
 void omap3isp_configure_bridge(struct isp_device *isp,
 			       enum ccdc_input_entity input,
 			       const struct isp_parallel_platform_data *pdata,
-			       unsigned int shift);
+			       unsigned int shift, unsigned int bridge);
 
 struct isp_device *omap3isp_get(struct isp_device *isp);
 void omap3isp_put(struct isp_device *isp);
diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index d5aed79..7e148c8 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -61,6 +61,8 @@ static const unsigned int ccdc_fmts[] = {
 	V4L2_MBUS_FMT_SRGGB12_1X12,
 	V4L2_MBUS_FMT_SBGGR12_1X12,
 	V4L2_MBUS_FMT_SGBRG12_1X12,
+	V4L2_MBUS_FMT_YUYV8_2X8,
+	V4L2_MBUS_FMT_UYVY8_2X8,
 };
 
 /*
@@ -973,8 +975,19 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 				unsigned int data_size)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
+	const struct v4l2_mbus_framefmt *format;
 	u32 syn_mode = ISPCCDC_SYN_MODE_VDHDEN;
 
+	format = &ccdc->formats[CCDC_PAD_SINK];
+
+	if (format->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+	    format->code == V4L2_MBUS_FMT_UYVY8_2X8) {
+		/* The bridge is enabled for YUV8 formats. Configure the input
+		 * mode accordingly.
+		 */
+		syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR16;
+	}
+
 	switch (data_size) {
 	case 8:
 		syn_mode |= ISPCCDC_SYN_MODE_DATSIZ_8;
@@ -1000,6 +1013,19 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
 
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
+
+	/* The CCDC_CFG.Y8POS bit is used in YCbCr8 input mode only. The
+	 * hardware seems to ignore it in all other input modes.
+	 */
+	if (format->code == V4L2_MBUS_FMT_UYVY8_2X8)
+		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_Y8POS);
+	else
+		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_Y8POS);
+
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
+		    ISPCCDC_REC656IF_R656ON);
 }
 
 /* CCDC formats descriptions */
@@ -1088,6 +1114,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	unsigned int depth_in = 0;
 	struct media_pad *pad;
 	unsigned long flags;
+	unsigned int bridge;
 	unsigned int shift;
 	u32 syn_mode;
 	u32 ccdc_pattern;
@@ -1098,7 +1125,9 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
 			->bus.parallel;
 
-	/* Compute shift value for lane shifter to configure the bridge. */
+	/* Compute the lane shifter shift value and enable the bridge when the
+	 * input format is YUV.
+	 */
 	fmt_src.pad = pad->index;
 	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
 	if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
@@ -1109,14 +1138,18 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	fmt_info = omap3isp_video_format_info
 		(isp->isp_ccdc.formats[CCDC_PAD_SINK].code);
 	depth_out = fmt_info->width;
-
 	shift = depth_in - depth_out;
-	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift);
 
-	ccdc_config_sync_if(ccdc, pdata, depth_out);
+	if (fmt_info->code == V4L2_MBUS_FMT_YUYV8_2X8)
+		bridge = ISPCTRL_PAR_BRIDGE_LENDIAN;
+	else if (fmt_info->code == V4L2_MBUS_FMT_UYVY8_2X8)
+		bridge = ISPCTRL_PAR_BRIDGE_BENDIAN;
+	else
+		bridge = ISPCTRL_PAR_BRIDGE_DISABLE;
 
-	/* CCDC_PAD_SINK */
-	format = &ccdc->formats[CCDC_PAD_SINK];
+	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift, bridge);
+
+	ccdc_config_sync_if(ccdc, pdata, depth_out);
 
 	syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
@@ -1135,13 +1168,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	else
 		syn_mode &= ~ISPCCDC_SYN_MODE_SDR2RSZ;
 
-	/* Use PACK8 mode for 1byte per pixel formats. */
-	if (omap3isp_video_format_info(format->code)->width <= 8)
-		syn_mode |= ISPCCDC_SYN_MODE_PACK8;
-	else
-		syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
-
-	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
+	/* CCDC_PAD_SINK */
+	format = &ccdc->formats[CCDC_PAD_SINK];
 
 	/* Mosaic filter */
 	switch (format->code) {
@@ -1172,6 +1200,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
 
 	/* CCDC_PAD_SOURCE_OF */
+	format = &ccdc->formats[CCDC_PAD_SOURCE_OF];
 	crop = &ccdc->crop;
 
 	isp_reg_writel(isp, (crop->left << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
@@ -1185,6 +1214,24 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 
 	ccdc_config_outlineoffset(ccdc, ccdc->video_out.bpl_value, 0, 0);
 
+	/* The CCDC outputs data in UYVY order by default. Swap bytes to get
+	 * YUYV.
+	 */
+	if (format->code == V4L2_MBUS_FMT_YUYV8_1X16)
+		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_BSWD);
+	else
+		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_BSWD);
+
+	/* Use PACK8 mode for 1byte per pixel formats. */
+	if (omap3isp_video_format_info(format->code)->width <= 8)
+		syn_mode |= ISPCCDC_SYN_MODE_PACK8;
+	else
+		syn_mode &= ~ISPCCDC_SYN_MODE_PACK8;
+
+	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
+
 	/* CCDC_PAD_SOURCE_VP */
 	format = &ccdc->formats[CCDC_PAD_SOURCE_VP];
 
@@ -1199,6 +1246,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		       (format->height << ISPCCDC_VP_OUT_VERT_NUM_SHIFT),
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VP_OUT);
 
+	/* Lens shading correction. */
 	spin_lock_irqsave(&ccdc->lsc.req_lock, flags);
 	if (ccdc->lsc.request == NULL)
 		goto unlock;
@@ -1776,8 +1824,8 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh,
 		unsigned int pad, struct v4l2_mbus_framefmt *fmt,
 		enum v4l2_subdev_format_whence which)
 {
-	struct v4l2_mbus_framefmt *format;
 	const struct isp_format_info *info;
+	enum v4l2_mbus_pixelcode pixelcode;
 	unsigned int width = fmt->width;
 	unsigned int height = fmt->height;
 	struct v4l2_rect *crop;
@@ -1785,9 +1833,6 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh,
 
 	switch (pad) {
 	case CCDC_PAD_SINK:
-		/* TODO: If the CCDC output formatter pad is connected directly
-		 * to the resizer, only YUV formats can be used.
-		 */
 		for (i = 0; i < ARRAY_SIZE(ccdc_fmts); i++) {
 			if (fmt->code == ccdc_fmts[i])
 				break;
@@ -1803,8 +1848,26 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh,
 		break;
 
 	case CCDC_PAD_SOURCE_OF:
-		format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, which);
-		memcpy(fmt, format, sizeof(*fmt));
+		pixelcode = fmt->code;
+		*fmt = *__ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, which);
+
+		/* YUV formats are converted from 2X8 to 1X16 by the bridge and
+		 * can be byte-swapped.
+		 */
+		if (fmt->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+		    fmt->code == V4L2_MBUS_FMT_UYVY8_2X8) {
+			/* Use the user requested format if YUV. */
+			if (pixelcode == V4L2_MBUS_FMT_YUYV8_2X8 ||
+			    pixelcode == V4L2_MBUS_FMT_UYVY8_2X8 ||
+			    pixelcode == V4L2_MBUS_FMT_YUYV8_1X16 ||
+			    pixelcode == V4L2_MBUS_FMT_UYVY8_1X16)
+				fmt->code = pixelcode;
+
+			if (fmt->code == V4L2_MBUS_FMT_YUYV8_2X8)
+				fmt->code = V4L2_MBUS_FMT_YUYV8_1X16;
+			else if (fmt->code == V4L2_MBUS_FMT_UYVY8_2X8)
+				fmt->code = V4L2_MBUS_FMT_UYVY8_1X16;
+		}
 
 		/* Hardcode the output size to the crop rectangle size. */
 		crop = __ccdc_get_crop(ccdc, fh, which);
@@ -1813,13 +1876,17 @@ ccdc_try_format(struct isp_ccdc_device *ccdc, struct v4l2_subdev_fh *fh,
 		break;
 
 	case CCDC_PAD_SOURCE_VP:
-		format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, which);
-		memcpy(fmt, format, sizeof(*fmt));
+		*fmt = *__ccdc_get_format(ccdc, fh, CCDC_PAD_SINK, which);
 
 		/* The video port interface truncates the data to 10 bits. */
 		info = omap3isp_video_format_info(fmt->code);
 		fmt->code = info->truncated;
 
+		/* YUV formats are not supported by the video port. */
+		if (fmt->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+		    fmt->code == V4L2_MBUS_FMT_UYVY8_2X8)
+			fmt->code = 0;
+
 		/* The number of lines that can be clocked out from the video
 		 * port output must be at least one line less than the number
 		 * of input lines.
@@ -1902,14 +1969,46 @@ static int ccdc_enum_mbus_code(struct v4l2_subdev *sd,
 		break;
 
 	case CCDC_PAD_SOURCE_OF:
+		format = __ccdc_get_format(ccdc, fh, code->pad,
+					   V4L2_SUBDEV_FORMAT_TRY);
+
+		if (format->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+		    format->code == V4L2_MBUS_FMT_UYVY8_2X8) {
+			/* In YUV mode the CCDC can swap bytes. */
+			if (code->index == 0)
+				code->code = V4L2_MBUS_FMT_YUYV8_1X16;
+			else if (code->index == 1)
+				code->code = V4L2_MBUS_FMT_UYVY8_1X16;
+			else
+				return -EINVAL;
+		} else {
+			/* In raw mode, no configurable format confversion is
+			 * available.
+			 */
+			if (code->index == 0)
+				code->code = format->code;
+			else
+				return -EINVAL;
+		}
+		break;
+
 	case CCDC_PAD_SOURCE_VP:
-		/* No format conversion inside CCDC */
+		/* The CCDC supports no configurable format conversion
+		 * compatible with the video port. Enumerate a single output
+		 * format code.
+		 */
 		if (code->index != 0)
 			return -EINVAL;
 
-		format = __ccdc_get_format(ccdc, fh, CCDC_PAD_SINK,
+		format = __ccdc_get_format(ccdc, fh, code->pad,
 					   V4L2_SUBDEV_FORMAT_TRY);
 
+		/* A pixel code equal to 0 means that the video port doesn't
+		 * support the input format. Don't enumerate any pixel code.
+		 */
+		if (format->code == 0)
+			return -EINVAL;
+
 		code->code = format->code;
 		break;
 
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index 9ce158e..f837b76 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -120,6 +120,10 @@ static struct isp_format_info formats[] = {
 	{ V4L2_MBUS_FMT_YUYV8_2X8, V4L2_MBUS_FMT_YUYV8_2X8,
 	  V4L2_MBUS_FMT_YUYV8_2X8, 0,
 	  V4L2_PIX_FMT_YUYV, 8, 16, },
+	/* Empty entry to catch the unsupported pixel code (0) used by the CCDC
+	 * module and avoid NULL pointer dereferences.
+	 */
+	{ 0, }
 };
 
 const struct isp_format_info *
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 5ab9449..9584269 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -42,12 +42,6 @@ enum isp_interface_type {
 };
 
 enum {
-	ISP_BRIDGE_DISABLE = 0,
-	ISP_BRIDGE_LITTLE_ENDIAN = 2,
-	ISP_BRIDGE_BIG_ENDIAN = 3,
-};
-
-enum {
 	ISP_LANE_SHIFT_0 = 0,
 	ISP_LANE_SHIFT_2 = 1,
 	ISP_LANE_SHIFT_4 = 2,
@@ -69,10 +63,6 @@ enum {
  *		0 - Active high, 1 - Active low
  * @data_pol: Data polarity
  *		0 - Normal, 1 - One's complement
- * @bridge: CCDC Bridge input control
- *		ISP_BRIDGE_DISABLE - Disable
- *		ISP_BRIDGE_LITTLE_ENDIAN - Little endian
- *		ISP_BRIDGE_BIG_ENDIAN - Big endian
  */
 struct isp_parallel_platform_data {
 	unsigned int data_lane_shift:2;
@@ -80,7 +70,6 @@ struct isp_parallel_platform_data {
 	unsigned int hs_pol:1;
 	unsigned int vs_pol:1;
 	unsigned int data_pol:1;
-	unsigned int bridge:2;
 };
 
 enum {
-- 
1.7.3.4

Re: [PATCH 6/6] omap3isp: ccdc: Add YUV input formats support

[Sakari Ailus]

Laurent Pinchart wrote:
> Enable the bridge automatically when the input format is YUYV8 or UYVY8.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks for the patch, Laurent!

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>

-- 
Sakari Ailus
sakari.ailus@iki.fi