[HACK v2 0/4] YUYV input support for the OMAP3 ISP

[HACK v2 0/4] YUYV input support for the OMAP3 ISP

[Laurent Pinchart]

Hi everybody,

Here's (already) a second version of this patch set. This first version was
missing a patch.

The set is based on
http://git.linuxtv.org/pinchartl/media.git/shortlog/refs/heads/omap3isp-omap3isp-next

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

 drivers/media/video/omap3isp/ispccdc.c  |  105 ++++++++++++++-----------------
 drivers/media/video/omap3isp/ispccdc.h  |   36 -----------
 drivers/media/video/omap3isp/ispvideo.c |   44 +++++++------
 drivers/media/video/omap3isp/ispvideo.h |    4 +-
 include/media/omap3isp.h                |    6 ++
 5 files changed, 82 insertions(+), 113 deletions(-)

-- 
Regards,

Laurent Pinchart

[HACK v2 1/4] 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 |   42 +++----------------------------
 drivers/media/video/omap3isp/ispccdc.h |   18 -------------
 2 files changed, 4 insertions(+), 56 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index 40b141c..176efae 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -965,13 +965,11 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
 				     ISPCCDC_SYN_MODE);
 
+	syn_mode &= ~(ISPCCDC_SYN_MODE_FLDSTAT | ISPCCDC_SYN_MODE_EXWEN |
+		      ISPCCDC_SYN_MODE_FLDMODE | ISPCCDC_SYN_MODE_FLDPOL |
+		      ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT);
 	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:
@@ -988,21 +986,11 @@ 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
@@ -1013,23 +1001,6 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 	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);
 
 	if (!syncif->bt_r656_en)
@@ -1148,6 +1119,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);
@@ -2258,13 +2230,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 483a19c..0e98f10 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

[HACK v2 2/4] 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  |    6 ++--
 drivers/media/video/omap3isp/ispvideo.c |   38 +++++++++++++++---------------
 drivers/media/video/omap3isp/ispvideo.h |    4 ++-
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index 176efae..8865741 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -1108,12 +1108,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);
@@ -1145,7 +1145,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;
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index ba86f11..1fd15c8 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -48,58 +48,58 @@
 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_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_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 *
@@ -143,7 +143,7 @@ static bool isp_video_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;
 }
 
 /*
diff --git a/drivers/media/video/omap3isp/ispvideo.h b/drivers/media/video/omap3isp/ispvideo.h
index 53160aa..b02a486 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;
 };
 
-- 
1.7.3.4

[HACK v2 3/4] 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 needs it.

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

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index 8865741..9efb703 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -623,9 +623,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->bpp) {
 	case 8:
 		return;
 
@@ -808,6 +811,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;
@@ -816,7 +820,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->bpp) {
 	case 8:
 	case 10:
 		fmtcfg_vp |= ISPCCDC_FMTCFG_VPIN_9_0;
@@ -954,12 +960,12 @@ 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 = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
@@ -971,7 +977,7 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 	syn_mode |= ISPCCDC_SYN_MODE_VDHDEN;
 
 	syn_mode &= ~ISPCCDC_SYN_MODE_DATSIZ_MASK;
-	switch (syncif->datsz) {
+	switch (data_size) {
 	case 8:
 		syn_mode |= ISPCCDC_SYN_MODE_DATSIZ_8;
 		break;
@@ -986,26 +992,22 @@ 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;
 	else
 		syn_mode &= ~ISPCCDC_SYN_MODE_DATAPOL;
 
-	if (syncif->hdpol)
+	if (pdata && pdata->hs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
 	else
 		syn_mode &= ~ISPCCDC_SYN_MODE_HDPOL;
 
-	if (syncif->vdpol)
+	if (pdata && pdata->vs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
 	else
 		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 */
@@ -1118,11 +1120,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];
@@ -2230,8 +2228,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 0e98f10..54811ce 100644
--- a/drivers/media/video/omap3isp/ispccdc.h
+++ b/drivers/media/video/omap3isp/ispccdc.h
@@ -47,22 +47,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;
-};
-
-/*
  * struct ispccdc_vp - Structure for Video Port parameters
  * @pixelclk: Input pixel clock in Hz
  */
@@ -143,7 +127,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
  * @vpcfg: Video port configuration
  * @underrun: A buffer underrun occurred and a new buffer has been queued
  * @state: Streaming state
@@ -173,7 +156,6 @@ struct isp_ccdc_device {
 	unsigned int update;
 	unsigned int shadow_update;
 
-	struct ispccdc_syncif syncif;
 	struct ispccdc_vp vpcfg;
 
 	unsigned int underrun:1;
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index e917b1d..5291665 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -63,6 +63,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
@@ -73,6 +75,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

[HACK v2 4/4] omap3isp: ccdc: Add YUV input formats support

[Laurent Pinchart]

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

diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
index 9efb703..3bc9b7d 100644
--- a/drivers/media/video/omap3isp/ispccdc.c
+++ b/drivers/media/video/omap3isp/ispccdc.c
@@ -57,6 +57,10 @@ 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_1X16,
+	V4L2_MBUS_FMT_UYVY8_1X16,
+	V4L2_MBUS_FMT_YUYV8_2X8,
+	V4L2_MBUS_FMT_UYVY8_2X8,
 };
 
 /*
@@ -628,7 +632,7 @@ static void ccdc_configure_alaw(struct isp_ccdc_device *ccdc)
 
 	info = omap3isp_video_format_info(ccdc->formats[CCDC_PAD_SINK].code);
 
-	switch (info->bpp) {
+	switch (info->width) {
 	case 8:
 		return;
 
@@ -822,7 +826,7 @@ static void ccdc_config_vp(struct isp_ccdc_device *ccdc)
 
 	info = omap3isp_video_format_info(ccdc->formats[CCDC_PAD_SINK].code);
 
-	switch (info->bpp) {
+	switch (info->width) {
 	case 8:
 	case 10:
 		fmtcfg_vp |= ISPCCDC_FMTCFG_VPIN_9_0;
@@ -968,6 +972,7 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 				unsigned int data_size)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
+	struct v4l2_mbus_framefmt *format;
 	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
 				     ISPCCDC_SYN_MODE);
 
@@ -976,6 +981,16 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 		      ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT);
 	syn_mode |= ISPCCDC_SYN_MODE_VDHDEN;
 
+	format = &ccdc->formats[CCDC_PAD_SINK];
+
+	syn_mode &= ~ISPCCDC_SYN_MODE_INPMOD_MASK;
+	if (format->code == V4L2_MBUS_FMT_YUYV8_2X8 ||
+	    format->code == V4L2_MBUS_FMT_UYVY8_2X8)
+		syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR8;
+	else if (format->code == V4L2_MBUS_FMT_YUYV8_1X16 ||
+		 format->code == V4L2_MBUS_FMT_UYVY8_1X16)
+		syn_mode |= ISPCCDC_SYN_MODE_INPMOD_YCBCR16;
+
 	syn_mode &= ~ISPCCDC_SYN_MODE_DATSIZ_MASK;
 	switch (data_size) {
 	case 8:
@@ -1008,6 +1023,20 @@ 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);
+
+	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);
+
+	if (pdata && pdata->bt656)
+		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
+			    ISPCCDC_REC656IF_R656ON);
+	else
+		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
+			    ISPCCDC_REC656IF_R656ON);
 }
 
 /* CCDC formats descriptions */
diff --git a/drivers/media/video/omap3isp/ispvideo.c b/drivers/media/video/omap3isp/ispvideo.c
index 1fd15c8..f88b8af 100644
--- a/drivers/media/video/omap3isp/ispvideo.c
+++ b/drivers/media/video/omap3isp/ispvideo.c
@@ -100,6 +100,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 *
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 5291665..2cedd44 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -69,6 +69,8 @@ enum {
  *		ISP_BRIDGE_DISABLE - Disable
  *		ISP_BRIDGE_LITTLE_ENDIAN - Little endian
  *		ISP_BRIDGE_BIG_ENDIAN - Big endian
+ * @bt656: ITU-R BT656 embedded synchronization
+ *		0 - HS/VS sync, 1 - BT656 sync
  */
 struct isp_parallel_platform_data {
 	unsigned int data_lane_shift:2;
@@ -77,6 +79,7 @@ struct isp_parallel_platform_data {
 	unsigned int vs_pol:1;
 	unsigned int data_pol:1;
 	unsigned int bridge:2;
+	unsigned int bt656:1;
 };
 
 enum {
-- 
1.7.3.4

Re: [HACK v2 1/4] omap3isp: ccdc: Remove support for interlaced data and master HS/VS mode

[Sakari Ailus]

Hi Laurent,

Thanks for the patch.

On Wed, Aug 31, 2011 at 06:31:59PM +0200, Laurent Pinchart wrote:
> 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 |   42 +++----------------------------
>  drivers/media/video/omap3isp/ispccdc.h |   18 -------------
>  2 files changed, 4 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/media/video/omap3isp/ispccdc.c b/drivers/media/video/omap3isp/ispccdc.c
> index 40b141c..176efae 100644
> --- a/drivers/media/video/omap3isp/ispccdc.c
> +++ b/drivers/media/video/omap3isp/ispccdc.c
> @@ -965,13 +965,11 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
>  	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
>  				     ISPCCDC_SYN_MODE);
>  
> +	syn_mode &= ~(ISPCCDC_SYN_MODE_FLDSTAT | ISPCCDC_SYN_MODE_EXWEN |
> +		      ISPCCDC_SYN_MODE_FLDMODE | ISPCCDC_SYN_MODE_FLDPOL |
> +		      ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT);

This might be a silly question but is there a need to read some bits from
the the register? It seems to me that awfully many of the will always get
overwritten.

>  	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:
> @@ -988,21 +986,11 @@ 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
> @@ -1013,23 +1001,6 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
>  	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);
>  
>  	if (!syncif->bt_r656_en)
> @@ -1148,6 +1119,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);
> @@ -2258,13 +2230,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 483a19c..0e98f10 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
> 

-- 
Sakari Ailus
sakari.ailus@iki.fi

Re: [HACK v2 1/4] omap3isp: ccdc: Remove support for interlaced data and master HS/VS mode

[Laurent Pinchart]

Hi Sakari,

On Wednesday 31 August 2011 21:47:11 Sakari Ailus wrote:
> On Wed, Aug 31, 2011 at 06:31:59PM +0200, Laurent Pinchart wrote:
> > 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 |   42
> >  +++---------------------------- drivers/media/video/omap3isp/ispccdc.h
> >  |   18 -------------
> >  2 files changed, 4 insertions(+), 56 deletions(-)
> > 
> > diff --git a/drivers/media/video/omap3isp/ispccdc.c
> > b/drivers/media/video/omap3isp/ispccdc.c index 40b141c..176efae 100644
> > --- a/drivers/media/video/omap3isp/ispccdc.c
> > +++ b/drivers/media/video/omap3isp/ispccdc.c
> > @@ -965,13 +965,11 @@ static void ccdc_config_sync_if(struct
> > isp_ccdc_device *ccdc,
> > 
> >  	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
> >  	
> >  				     ISPCCDC_SYN_MODE);
> > 
> > +	syn_mode &= ~(ISPCCDC_SYN_MODE_FLDSTAT | ISPCCDC_SYN_MODE_EXWEN |
> > +		      ISPCCDC_SYN_MODE_FLDMODE | ISPCCDC_SYN_MODE_FLDPOL |
> > +		      ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT);
> 
> This might be a silly question but is there a need to read some bits from
> the the register? It seems to me that awfully many of the will always get
> overwritten.

I've been thinking about that as well. The only bits we won't overwrite are 
the reserved bits. I guess we can just write them to 0.

> >  	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:
> > @@ -988,21 +986,11 @@ 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
> > 
> > @@ -1013,23 +1001,6 @@ static void ccdc_config_sync_if(struct
> > isp_ccdc_device *ccdc,
> > 
> >  	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);
> >  	
> >  	if (!syncif->bt_r656_en)
> > 
> > @@ -1148,6 +1119,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);
> > 
> > @@ -2258,13 +2230,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 483a19c..0e98f10 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;
> >  
> >  };

-- 
Regards,

Laurent Pinchart

Re: [HACK v2 1/4] omap3isp: ccdc: Remove support for interlaced data and master HS/VS mode

[Sakari Ailus]

On Wed, Aug 31, 2011 at 09:58:42PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 31 August 2011 21:47:11 Sakari Ailus wrote:
> > On Wed, Aug 31, 2011 at 06:31:59PM +0200, Laurent Pinchart wrote:
> > > 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 |   42
> > >  +++---------------------------- drivers/media/video/omap3isp/ispccdc.h
> > >  |   18 -------------
> > >  2 files changed, 4 insertions(+), 56 deletions(-)
> > > 
> > > diff --git a/drivers/media/video/omap3isp/ispccdc.c
> > > b/drivers/media/video/omap3isp/ispccdc.c index 40b141c..176efae 100644
> > > --- a/drivers/media/video/omap3isp/ispccdc.c
> > > +++ b/drivers/media/video/omap3isp/ispccdc.c
> > > @@ -965,13 +965,11 @@ static void ccdc_config_sync_if(struct
> > > isp_ccdc_device *ccdc,
> > > 
> > >  	u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
> > >  	
> > >  				     ISPCCDC_SYN_MODE);
> > > 
> > > +	syn_mode &= ~(ISPCCDC_SYN_MODE_FLDSTAT | ISPCCDC_SYN_MODE_EXWEN |
> > > +		      ISPCCDC_SYN_MODE_FLDMODE | ISPCCDC_SYN_MODE_FLDPOL |
> > > +		      ISPCCDC_SYN_MODE_FLDOUT | ISPCCDC_SYN_MODE_VDHDOUT);
> > 
> > This might be a silly question but is there a need to read some bits from
> > the the register? It seems to me that awfully many of the will always get
> > overwritten.
> 
> I've been thinking about that as well. The only bits we won't overwrite are 
> the reserved bits. I guess we can just write them to 0.

I agree. The read can be removed, and that actually simplifies the rest of
the function as well since there's no need to reset those bits anymore.

> > >  	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:
> > > @@ -988,21 +986,11 @@ 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
> > > 
> > > @@ -1013,23 +1001,6 @@ static void ccdc_config_sync_if(struct
> > > isp_ccdc_device *ccdc,
> > > 
> > >  	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);
> > >  	
> > >  	if (!syncif->bt_r656_en)
> > > 
> > > @@ -1148,6 +1119,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);
> > > 
> > > @@ -2258,13 +2230,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 483a19c..0e98f10 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;
> > >  
> > >  };
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Sakari Ailus
sakari.ailus@iki.fi