OMAP3ISP won't start

DISCUSSION Gary Thomas 2 participants 8 messages Apr 2, 2012 Media (V4L/DVB)

Share & tools

Gary Thomas

author

2012-04-02 12:24 UTC #0 lore raw

I'm testing Laurent's tree with the BT656 support. My system has an OMAP/3530 + TVP5150 running in BT656 mode. Sadly, this isn't working - I only get an endless stream of these messages

  [   43.457427] omap3isp omap3isp: CCDC won't become idle!
  [   43.490814] omap3isp omap3isp: CCDC won't become idle!

I've compared the operation between a similarly patched 3.2 system and the current 3.3. All OMAP3 CCDC registers are identical, as well as the TVP5150 registers at the time that streaming is enabled.

Any ideas what might be wrong? where else to look?

Thanks

Just to be clear - I did have to make a few patches to both the TVP5150 driver and CCDC part of OMAP3ISP as some of the BT656 support is still not in Laurent's tree. I'll send patches, etc, once I get it working, but as mentioned above, at least at the register level, these are set up the same as in my working tree.

Signature · 4 lines

------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Enrico

2012-04-02 12:32 UTC #1 lore raw

Quoted 5 lines · Gary Thomas

Just to be clear - I did have to make a few patches to both the TVP5150 driver and CCDC part of OMAP3ISP as some of the BT656 support is still not in Laurent's tree. I'll send patches, etc, once I get it working, but as mentioned above, at least at the register level, these are set up the same as in my working tree.

What patches did you add on top of Laurent tree?

I had a look some days ago and for example it's missing the right setup for VD0/VD1 (no VD1 and VD0 set to half height), this could be the cause of a not working isp. You can check if you get interrupts from the isp (/proc/interrupts).

Another thing that is missing is the logic in the irq handler that wait for a complete frame before calling next video buffer (or something like that).

Enrico

Gary Thomas

author

2012-04-02 12:45 UTC #2 lore raw

Quoted 7 lines · Enrico

Quoted 5 lines · Gary Thomas
Just to be clear - I did have to make a few patches to both the TVP5150 driver and CCDC part of OMAP3ISP as some of the BT656 support is still not in Laurent's tree. I'll send patches, etc, once I get it working, but as mentioned above, at least at the register level, these are set up the same as in my working tree.

Quoted 6 lines

What patches did you add on top of Laurent tree?
I had a look some days ago and for example it's missing the right setup for VD0/VD1 (no VD1 and VD0 set to half height), this could be the cause of a not working isp. You can check if you get interrupts from the isp (/proc/interrupts).

Getting lots of interrupts:
24: 3376 INTC omap-iommu.0, OMAP3 ISP

Quoted 4 lines

Another thing that is missing is the logic in the irq handler that wait for a complete frame before calling next video buffer (or something like that).

The items you mention are just what I merged from my previous kernel. My changes are still pretty rough but I can send them to you if you'd like.

Signature · 4 lines

------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Enrico

2012-04-02 13:15 UTC #3 lore raw

Quoted 3 lines · Gary Thomas

The items you mention are just what I merged from my previous kernel. My changes are still pretty rough but I can send them to you if you'd like.

Post them here and we will try to spot where the problem is, they could be useful for Laurent too as a reference.

Enrico

Gary Thomas

author

2012-04-02 13:31 UTC #4 lore raw

Quoted 7 lines · Enrico

Quoted 3 lines · Gary Thomas
The items you mention are just what I merged from my previous kernel. My changes are still pretty rough but I can send them to you if you'd like.
Post them here and we will try to spot where the problem is, they could be useful for Laurent too as a reference.

Attached.

Signature · 4 lines

------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

drivers/media/video/omap3isp/ispccdc.c +117 ~~-25~~

index f62a38b..ab2717b 100644
@@ -62,6 +62,8 @@ static const unsigned int ccdc_fmts[] = {
 	V4L2_MBUS_FMT_UYVY8_2X8,
 };
 
+static bool ccdc_input_is_bt656(struct isp_ccdc_device *ccdc);
+
 /*
  * ccdc_print_status - Print current CCDC Module register values.
  * @ccdc: Pointer to ISP CCDC device.

@@ -794,11 +796,16 @@ static void ccdc_apply_controls(struct isp_ccdc_device *ccdc)
 void omap3isp_ccdc_restore_context(struct isp_device *isp)
 {
 	struct isp_ccdc_device *ccdc = &isp->isp_ccdc;
+	struct v4l2_mbus_framefmt *format;
 
 	isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG, ISPCCDC_CFG_VDLC);
 
-	ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF
-		     | OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
+	/* CCDC_PAD_SINK */
+	format = &ccdc->formats[CCDC_PAD_SINK];
+	if ((format->code != V4L2_MBUS_FMT_UYVY8_2X8) &&
+            (format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc->update = OMAP3ISP_CCDC_ALAW | OMAP3ISP_CCDC_LPF |
+			       OMAP3ISP_CCDC_BLCLAMP | OMAP3ISP_CCDC_BCOMP;
 	ccdc_apply_controls(ccdc);
 	ccdc_configure_fpc(ccdc);
 }

@@ -1012,6 +1019,9 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 	if (pdata && pdata->vs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
 
+	if (pdata && pdata->fldmode)
+		syn_mode |= ISPCCDC_SYN_MODE_FLDMODE;
+
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
 	if (format->code == V4L2_MBUS_FMT_UYVY8_2X8)

@@ -1023,10 +1033,10 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 
 	if (pdata && pdata->bt656)
 		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 	else
 		isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_REC656IF,
-			    ISPCCDC_REC656IF_R656ON);
+			    ISPCCDC_REC656IF_R656ON | ISPCCDC_REC656IF_ECCFVH);
 }
 
 /* CCDC formats descriptions */

@@ -1108,6 +1118,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	struct isp_parallel_platform_data *pdata = NULL;
 	struct v4l2_subdev *sensor;
 	struct v4l2_mbus_framefmt *format;
+	struct v4l2_pix_format pix;
 	const struct isp_format_info *fmt_info;
 	struct v4l2_subdev_format fmt_src;
 	unsigned int depth_out;

@@ -1166,6 +1177,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	/* CCDC_PAD_SINK */
 	format = &ccdc->formats[CCDC_PAD_SINK];
 
+	if (format->code == V4L2_MBUS_FMT_UYVY8_2X8)
+		isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_CFG,
+			    ISPCCDC_CFG_Y8POS);
+
 	/* Mosaic filter */
 	switch (format->code) {
 	case V4L2_MBUS_FMT_SRGGB10_1X10:

@@ -1185,28 +1200,70 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		ccdc_pattern = ccdc_sgrbg_pattern;
 		break;
 	}
-	ccdc_config_imgattr(ccdc, ccdc_pattern);
 
-	/* Generate VD0 on the last line of the image and VD1 on the
-	 * 2/3 height line.
-	 */
-	isp_reg_writel(isp, ((format->height - 2) << ISPCCDC_VDINT_0_SHIFT) |
-		       ((format->height * 2 / 3) << ISPCCDC_VDINT_1_SHIFT),
-		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
+	if ((format->code != V4L2_MBUS_FMT_YUYV8_2X8) &&
+			(format->code != V4L2_MBUS_FMT_UYVY8_2X8))
+		ccdc_config_imgattr(ccdc, ccdc_pattern);
+
+	/* BT656: Generate VD0 on the last line of each field, and we
+	* don't use VD1.
+	* Non BT656: Generate VD0 on the last line of the image and VD1 on the
+	* 2/3 height line.
+	*/
+	if (pdata->bt656)
+		isp_reg_writel(isp,
+			(format->height/2 - 2) << ISPCCDC_VDINT_0_SHIFT,
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
+	else
+		isp_reg_writel(isp,
+			((format->height - 2) << ISPCCDC_VDINT_0_SHIFT) |
+			((format->height * 2 / 3) << ISPCCDC_VDINT_1_SHIFT),
+			OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VDINT);
 
 	/* CCDC_PAD_SOURCE_OF */
 	format = &ccdc->formats[CCDC_PAD_SOURCE_OF];
 
-	isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
-		       ((format->width - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT),
-		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
+	isp_video_mbus_to_pix(&ccdc->video_out, format, &pix);
+
+	/* For BT656 the number of bytes would be width*2 */
+	if (pdata->bt656)
+		isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
+                               ((pix.bytesperline - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT),
+                               OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
+	else
+		isp_reg_writel(isp, (0 << ISPCCDC_HORZ_INFO_SPH_SHIFT) |
+                               ((format->width - 1) << ISPCCDC_HORZ_INFO_NPH_SHIFT),
+                               OMAP3_ISP_IOMEM_CCDC, ISPCCDC_HORZ_INFO);
 	isp_reg_writel(isp, 0 << ISPCCDC_VERT_START_SLV0_SHIFT,
 		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_START);
-	isp_reg_writel(isp, (format->height - 1)
-			<< ISPCCDC_VERT_LINES_NLV_SHIFT,
-		       OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
 
-	ccdc_config_outlineoffset(ccdc, ccdc->video_out.bpl_value, 0, 0);
+	if (pdata->bt656)
+		isp_reg_writel(isp, ((format->height >> 1) - 1) << ISPCCDC_VERT_LINES_NLV_SHIFT,
+                               OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
+	else
+		isp_reg_writel(isp, (format->height - 1) << ISPCCDC_VERT_LINES_NLV_SHIFT,
+                               OMAP3_ISP_IOMEM_CCDC, ISPCCDC_VERT_LINES);
+
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST0_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST1_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST2_SHIFT);
+	isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+		    ISPCCDC_SDOFST_LOFST_MASK << ISPCCDC_SDOFST_LOFST3_SHIFT);
+
+	/* In case of BT656 each alternate line must be stored into memory */
+	if (pdata->bt656) {
+		ccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENEVEN, 1);
+		ccdc_config_outlineoffset(ccdc, pix.bytesperline, EVENODD, 1);
+		ccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDEVEN, 1);
+		ccdc_config_outlineoffset(ccdc, pix.bytesperline, ODDODD, 1);
+                isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+                            ISPCCDC_SDOFST_FINV);
+	} else {
+		ccdc_config_outlineoffset(ccdc, ccdc->video_out.bpl_value, 0, 0);
+	}
 
 	/* CCDC_PAD_SOURCE_VP */
 	format = &ccdc->formats[CCDC_PAD_SOURCE_VP];

@@ -1253,6 +1310,11 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 unlock:
 	spin_unlock_irqrestore(&ccdc->lsc.req_lock, flags);
 
+	if (pdata->bt656)
+		ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	else
+		ccdc->update = 0;
+
 	ccdc_apply_controls(ccdc);
 }

@@ -1264,6 +1326,7 @@ static void __ccdc_enable(struct isp_ccdc_device *ccdc, int enable)
 			ISPCCDC_PCR_EN, enable ? ISPCCDC_PCR_EN : 0);
 }
 
+static int __ccdc_handle_stopping(struct isp_ccdc_device *ccdc, u32 event);
 static int ccdc_disable(struct isp_ccdc_device *ccdc)
 {
 	unsigned long flags;

@@ -1274,6 +1337,11 @@ static int ccdc_disable(struct isp_ccdc_device *ccdc)
 		ccdc->stopping = CCDC_STOP_REQUEST;
 	spin_unlock_irqrestore(&ccdc->lock, flags);
 
+	__ccdc_lsc_enable(ccdc, 0);
+	__ccdc_enable(ccdc, 0);
+	ccdc->stopping = CCDC_STOP_EXECUTED;
+	__ccdc_handle_stopping(ccdc, CCDC_STOP_FINISHED);
+
 	ret = wait_event_timeout(ccdc->wait,
 				 ccdc->stopping == CCDC_STOP_FINISHED,
 				 msecs_to_jiffies(2000));

@@ -1529,9 +1597,31 @@ static void ccdc_vd0_isr(struct isp_ccdc_device *ccdc)
 {
 	unsigned long flags;
 	int restart = 0;
-
-	if (ccdc->output & CCDC_OUTPUT_MEMORY)
-		restart = ccdc_isr_buffer(ccdc);
+	struct isp_device *isp = to_isp_device(ccdc);
+ 
+	if (ccdc->output & CCDC_OUTPUT_MEMORY) {
+		if (ccdc_input_is_bt656(ccdc)) {
+			u32 fid;
+			u32 syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC,
+					ISPCCDC_SYN_MODE);
+
+			fid = (syn_mode & ISPCCDC_SYN_MODE_FLDSTAT) == 0;
+			/* toggle the software maintained fid */
+			ccdc->fldstat ^= 1;
+
+			if (fid == ccdc->fldstat) {
+				if (fid == 0) {
+					restart = ccdc_isr_buffer(ccdc);
+					goto done;
+				}
+			} else if (fid == 0) {
+				ccdc->fldstat = fid;
+			}
+		} else {
+			restart = ccdc_isr_buffer(ccdc);
+		}
+	}
+done:
 
 	spin_lock_irqsave(&ccdc->lock, flags);
 	if (__ccdc_handle_stopping(ccdc, CCDC_EVENT_VD0)) {

@@ -1618,7 +1708,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (ccdc->state == ISP_PIPELINE_STREAM_STOPPED)
 		return 0;
 
-	if (events & IRQ0STATUS_CCDC_VD1_IRQ)
+	if (!ccdc_input_is_bt656(ccdc) &&
+            (events & IRQ0STATUS_CCDC_VD1_IRQ))
 		ccdc_vd1_isr(ccdc);
 
 	ccdc_lsc_isr(ccdc, events);

@@ -1626,7 +1717,8 @@ int omap3isp_ccdc_isr(struct isp_ccdc_device *ccdc, u32 events)
 	if (events & IRQ0STATUS_CCDC_VD0_IRQ)
 		ccdc_vd0_isr(ccdc);
 
-	if (events & IRQ0STATUS_HS_VS_IRQ)
+	if (!ccdc_input_is_bt656(ccdc) &&
+            (events & IRQ0STATUS_HS_VS_IRQ))
 		ccdc_hs_vs_isr(ccdc);
 
 	return 0;

@@ -1740,7 +1832,7 @@ static int ccdc_set_stream(struct v4l2_subdev *sd, int enable)
 		 * links are inactive.
 		 */
 		ccdc_config_vp(ccdc);
-		ccdc_enable_vp(ccdc, 1);
+		ccdc_enable_vp(ccdc, 0);
 		ccdc_print_status(ccdc);
 	}

@@ -2289,7 +2381,7 @@ int omap3isp_ccdc_init(struct isp_device *isp)
 
 	ccdc->vpcfg.pixelclk = 0;
 
-	ccdc->update = OMAP3ISP_CCDC_BLCLAMP;
+	ccdc->update = 0;
 	ccdc_apply_controls(ccdc);
 
 	ret = ccdc_init_entities(ccdc);

drivers/media/video/omap3isp/ispccdc.h +1 -0

index 836439a..2d31250 100644
@@ -157,6 +157,7 @@ struct isp_ccdc_device {
 	struct ispccdc_vp vpcfg;
 
 	unsigned int underrun:1;
+	unsigned int fldstat:1;
 	enum isp_pipeline_stream_state state;
 	spinlock_t lock;
 	wait_queue_head_t wait;

drivers/media/video/omap3isp/ispreg.h +1 -0

index 084ea77..f9c2e47 100644
@@ -824,6 +824,7 @@
 #define ISPCCDC_SDOFST_LOFST2_SHIFT		3
 #define ISPCCDC_SDOFST_LOFST1_SHIFT		6
 #define ISPCCDC_SDOFST_LOFST0_SHIFT		9
+#define ISPCCDC_SDOFST_LOFST_MASK               0x7
 #define EVENEVEN				1
 #define ODDEVEN					2
 #define EVENODD					3

drivers/media/video/omap3isp/ispvideo.c +3 -3

index d1526ab..9ee0a80 100644
@@ -165,9 +165,9 @@ static bool isp_video_is_shiftable(enum v4l2_mbus_pixelcode in,
  *
  * Return the number of padding bytes at end of line.
  */
-static unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
-					  const struct v4l2_mbus_framefmt *mbus,
-					  struct v4l2_pix_format *pix)
+unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
+                                   const struct v4l2_mbus_framefmt *mbus,
+                                   struct v4l2_pix_format *pix)
 {
 	unsigned int bpl = pix->bytesperline;
 	unsigned int min_bpl;

drivers/media/video/omap3isp/ispvideo.h +3 -0

index 8572e64..8bab73f 100644
@@ -206,6 +206,9 @@ void omap3isp_video_unregister(struct isp_video *video);
 struct isp_buffer *omap3isp_video_buffer_next(struct isp_video *video);
 void omap3isp_video_resume(struct isp_video *video, int continuous);
 struct media_pad *omap3isp_video_remote_pad(struct isp_video *video);
+extern unsigned int isp_video_mbus_to_pix(const struct isp_video *video,
+                                          const struct v4l2_mbus_framefmt *mbus,
+                                          struct v4l2_pix_format *pix);
 
 const struct isp_format_info *
 omap3isp_video_format_info(enum v4l2_mbus_pixelcode code);

drivers/media/video/tvp5150.c +180 -2

261 lines not on this page · raw message

include/media/omap3isp.h +3 -0

index f67b8c1..e1feb13 100644
@@ -61,6 +61,8 @@ enum {
  *		0 - Normal, 1 - One's complement
  * @bt656: ITU-R BT656 embedded synchronization
  *		0 - HS/VS sync, 1 - BT656 sync
+ * @fldmode: Field mode
+ *             0 - progressive, 1 - Interlaced
  */
 struct isp_parallel_platform_data {
 	unsigned int data_lane_shift:2;

@@ -69,6 +71,7 @@ struct isp_parallel_platform_data {
 	unsigned int vs_pol:1;
 	unsigned int data_pol:1;
 	unsigned int bt656:1;
+	unsigned int fldmode:1;
 };
 
 enum {

Enrico

2012-04-02 14:38 UTC #5 lore raw

Quoted 9 lines · Gary Thomas

Quoted 8 lines · Enrico
Quoted 4 lines · Gary Thomas
The items you mention are just what I merged from my previous kernel. My changes are still pretty rough but I can send them to you if you'd like.

Quoted 5 lines

Quoted 2 lines
Post them here and we will try to spot where the problem is, they could be useful for Laurent too as a reference.
Attached.

I just had a quick look and it seems everything is there, i can't test it right now but when i did test a mainline 3.3 kernel with my patches i had to use the "nohlt" kernel parm. If i'm not wrong without that param i had the same error, you can give it a try.

Enrico

Gary Thomas

author

2012-04-02 14:55 UTC #6 lore raw

Quoted 14 lines · Enrico

Quoted 13 lines · Gary Thomas
Quoted 10 lines · Enrico
Quoted 4 lines · Gary Thomas
The items you mention are just what I merged from my previous kernel. My changes are still pretty rough but I can send them to you if you'd like.
Post them here and we will try to spot where the problem is, they could be useful for Laurent too as a reference.

Quoted 6 lines

Quoted 1 line
Attached.
I just had a quick look and it seems everything is there, i can't test it right now but when i did test a mainline 3.3 kernel with my patches i had to use the "nohlt" kernel parm. If i'm not wrong without that param i had the same error, you can give it a try.

Hurray, that does work.

Signature · 4 lines

------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Enrico

2012-04-02 15:51 UTC #7 lore raw

Quoted 11 lines · Gary Thomas

Quoted 7 lines · Enrico
Quoted 1 line
Attached.
I just had a quick look and it seems everything is there, i can't test it right now but when i did test a mainline 3.3 kernel with my patches i had to use the "nohlt" kernel parm. If i'm not wrong without that param i had the same error, you can give it a try.
Hurray, that does work.

Laurent, this is an old problem that i noticed some time ago [1] but apparently things are getting worse.

The situation now is:

kernel 3.2 without cpu_idle: no need for nohlt kernel 3.2 with cpu_idle: nohlt is needed kernel 3.3 with/without cpu_idle: nohlt always needed

I really don't know where the problem comes from, but i verified that the cam powerdomain is correctly "acquired" during capture, i don't know what else i can check.

[1]: http://www.digipedia.pl/usenet/thread/18550/20510/

Enrico