Re: [PATCH] omap3isp: Add support for interlaced input data

2013-01-14 Thread William Swanson

On 01/09/2013 02:35 PM, Laurent Pinchart wrote:

On Tuesday 08 January 2013 14:49:41 William Swanson wrote:

I believe the data is combined in a single buffer, with alternate fields
interleaved.


Could you please double-check that ? I'd like to be sure, not just believe :-)


Sorry for the delay in getting back to you. I have checked it, and the 
fields are indeed interlaced into a single buffer. On the other hand, 
looking at this caused me to discover another problem with the patch.


According to the TI documentation, the CCDC_SDOFST register controls the 
deinterlacing process. My patch never configures this register, however, 
which is surprising. The reason things work on our boards is because we 
are carrying a separate patch which changes the register by accident. 
Oops! I have fixed this, and will be sending another patch with the 
CCDC_SDOFST changes.



In that case the OMAP3 ISP driver should set the v4l2_pix_format::field to
V4L2_FIELD_INTERLACED in the format-related ioctl when an interlaced format is
used. I suppose this could be added later - Sakari, any opinion ?


I don't have a lot of time to work on this stuff, so my main focus is 
just getting the data to flow. Changing the output format information 
involves other parts of the driver that I am not familiar with, so I 
don't know if I will be able to work on that bit.


By the way, thanks for taking the time to review this, Laurent.

-William
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] omap3isp: Add support for interlaced input data

2013-01-14 Thread William Swanson
If the remote video sensor reports an interlaced video mode, the CCDC block
should configure itself appropriately.

This patch reintroduces code with was removed in commit
cf7a3d91ade6c56bfd860b377f84bd58132f7a81, but in a way that is compatible
with the new media pipeline work.

This patch also cleans up the ccdc_config_outlineoffset function, which was
exposing too may low-level configuration bits. The only useful register
settings correspond to deinterlacing the video and flipping the image
vertically. Vertical flipping isn't used, so the function simply exposes
a boolean flag to enable deinterlacing. A flag for vertical flipping could
be added later.

Signed-off-by: William Swanson william.swan...@fuel7.com
---
 drivers/media/platform/omap3isp/ispccdc.c |   57 +++--
 drivers/media/platform/omap3isp/ispreg.h  |4 --
 include/media/omap3isp.h  |3 ++
 3 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
b/drivers/media/platform/omap3isp/ispccdc.c
index 60e60aa..7b795e6e 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -876,19 +876,15 @@ static void ccdc_enable_vp(struct isp_ccdc_device *ccdc, 
u8 enable)
  * ccdc_config_outlineoffset - Configure memory saving output line offset
  * @ccdc: Pointer to ISP CCDC device.
  * @offset: Address offset to start a new line. Must be twice the
- *  Output width and aligned on 32 byte boundary
- * @oddeven: Specifies the odd/even line pattern to be chosen to store the
- *   output.
- * @numlines: Set the value 0-3 for +1-4lines, 4-7 for -1-4lines.
+ *  output width and aligned on 32 byte boundary
+ * @interlaced: Write alternate frames to memory with an even/odd pattern.
  *
  * - Configures the output line offset when stored in memory
- * - Sets the odd/even line pattern to store the output
- *(EVENEVEN (1), ODDEVEN (2), EVENODD (3), ODDODD (4))
  * - Configures the number of even and odd line fields in case of rearranging
  * the lines.
  */
 static void ccdc_config_outlineoffset(struct isp_ccdc_device *ccdc,
-   u32 offset, u8 oddeven, u8 numlines)
+   u32 offset, bool interlaced)
 {
struct isp_device *isp = to_isp_device(ccdc);
 
@@ -901,25 +897,18 @@ static void ccdc_config_outlineoffset(struct 
isp_ccdc_device *ccdc,
isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
ISPCCDC_SDOFST_FOFST_4L);
 
-   switch (oddeven) {
-   case EVENEVEN:
+   if (interlaced) {
isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
-   (numlines  0x7)  ISPCCDC_SDOFST_LOFST0_SHIFT);
-   break;
-   case ODDEVEN:
-   isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
-   (numlines  0x7)  ISPCCDC_SDOFST_LOFST1_SHIFT);
-   break;
-   case EVENODD:
-   isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
-   (numlines  0x7)  ISPCCDC_SDOFST_LOFST2_SHIFT);
-   break;
-   case ODDODD:
-   isp_reg_set(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
-   (numlines  0x7)  ISPCCDC_SDOFST_LOFST3_SHIFT);
-   break;
-   default:
-   break;
+   1  ISPCCDC_SDOFST_LOFST0_SHIFT |
+   1  ISPCCDC_SDOFST_LOFST1_SHIFT |
+   1  ISPCCDC_SDOFST_LOFST2_SHIFT |
+   1  ISPCCDC_SDOFST_LOFST3_SHIFT );
+   } else {
+   isp_reg_clr(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SDOFST,
+   0x7  ISPCCDC_SDOFST_LOFST0_SHIFT |
+   0x7  ISPCCDC_SDOFST_LOFST1_SHIFT |
+   0x7  ISPCCDC_SDOFST_LOFST2_SHIFT |
+   0x7  ISPCCDC_SDOFST_LOFST3_SHIFT );
}
 }
 
@@ -970,10 +959,11 @@ void omap3isp_ccdc_max_rate(struct isp_ccdc_device *ccdc,
  * @ccdc: Pointer to ISP CCDC device.
  * @pdata: Parallel interface platform data (may be NULL)
  * @data_size: Data size
+ * @interlaced: Use interlaced mode instead of progressive mode
  */
 static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
struct isp_parallel_platform_data *pdata,
-   unsigned int data_size)
+   unsigned int data_size, bool interlaced)
 {
struct isp_device *isp = to_isp_device(ccdc);
const struct v4l2_mbus_framefmt *format;
@@ -1004,9 +994,15 @@ static void ccdc_config_sync_if(struct isp_ccdc_device 
*ccdc,
break;
}
 
+   if (interlaced)
+   syn_mode |= ISPCCDC_SYN_MODE_FLDMODE;
+
if (pdata  pdata-data_pol)
syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
 
+   if (pdata  pdata

Re: [PATCH] omap3isp: Add support for interlaced input data

2013-01-08 Thread William Swanson

On 01/07/2013 04:20 AM, Laurent Pinchart wrote:

What do you get in the memory buffers ? Are fields captured in separate
buffers or combined in a single buffer ? If they're combined, are they
interleaved or sequential in memory ?


I believe the data is combined in a single buffer, with alternate fields 
interleaved.


-William
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] omap3isp: Add support for interlaced input data

2013-01-04 Thread William Swanson

Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu:

On Monday 17 December 2012 18:12:19 William Swanson wrote:

If the remote video sensor reports an interlaced video mode, the CCDC block
should configure itself appropriately.


What will the CCDC do in that case ? Will it capture fields or frames to
memory ? If frames, what's the field layout ? You will most likely need to
modify ispvideo.c as well, to support interlacing in the V4L2 API, and
possibly add interlaced formats support to the media bus API.


Sorry for the delay in responding; today is my first day back at the 
office. I do not know the answers to these questions, and the 
documentation doesn't discuss interlacing much. Our application has the 
following pipeline:


composite video - TVP5146 decoder
- CCDC parallel interface - memory - application

One of the wires in the parallel interface, cam_fld, indicates the 
current field, and this patch simply enables that wire. Without the 
patch, every other line in our memory buffer is garbage; with the patch, 
the image comes out correctly.


As a matter of fact, an earlier version of the ISP driver actually 
contained code for dealing with this flag; it was removed in 
cf7a3d91ade6c56bfd860b377f84bd58132f7a81 along with a bunch of other 
cleanup work. This patch simply adds the code back, but in a way that is 
compatible with the new media pipeline stuff.


I believe that the CCDC simply captures image data a line at a time and 
writes it directly to memory, at least in our use case. The CCDC_SDOFST
register controls the layout, and the default value (which is what the 
driver uses now) is basically correct. I am not familiar enough with the 
V4L2 architecture to tell you how the driver decides that it now has a 
complete frame, or what that even means in an interlaced case.


On 12/27/2012 12:27 PM, Mauro Carvalho Chehab wrote:
 Btw, you missed to add a Signed-off-by: line on it.

Oops, this was a problem with my git setup. Both email addresses are 
mine; I can re-send the patch with them both set to the same address if 
you would prefer that.


-William
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] omap3isp: Add support for interlaced input data

2013-01-04 Thread William Swanson
If the remote video sensor reports an interlaced video mode, the CCDC block
should configure itself appropriately.

This patch reintroduces code with was removed in commit
cf7a3d91ade6c56bfd860b377f84bd58132f7a81, but in a way that is compatible
with the new media pipeline work.

Signed-off-by: William Swanson william.swan...@fuel7.com
---
 drivers/media/platform/omap3isp/ispccdc.c |   16 ++--
 include/media/omap3isp.h  |3 +++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
b/drivers/media/platform/omap3isp/ispccdc.c
index 60e60aa..5443ef4 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -970,10 +970,11 @@ void omap3isp_ccdc_max_rate(struct isp_ccdc_device *ccdc,
  * @ccdc: Pointer to ISP CCDC device.
  * @pdata: Parallel interface platform data (may be NULL)
  * @data_size: Data size
+ * @interlaced: Use interlaced mode instead of progressive mode
  */
 static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
struct isp_parallel_platform_data *pdata,
-   unsigned int data_size)
+   unsigned int data_size, bool interlaced)
 {
struct isp_device *isp = to_isp_device(ccdc);
const struct v4l2_mbus_framefmt *format;
@@ -1004,9 +1005,15 @@ static void ccdc_config_sync_if(struct isp_ccdc_device 
*ccdc,
break;
}
 
+   if (interlaced)
+   syn_mode |= ISPCCDC_SYN_MODE_FLDMODE;
+
if (pdata  pdata-data_pol)
syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
 
+   if (pdata  pdata-fld_pol)
+   syn_mode |= ISPCCDC_SYN_MODE_FLDPOL;
+
if (pdata  pdata-hs_pol)
syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
 
@@ -,6 +1118,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
const struct v4l2_rect *crop;
const struct isp_format_info *fmt_info;
struct v4l2_subdev_format fmt_src;
+   bool src_interlaced = false;
unsigned int depth_out;
unsigned int depth_in = 0;
struct media_pad *pad;
@@ -1132,6 +1140,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
fmt_src.pad = pad-index;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, fmt_src)) {
+   if (fmt_src.format.field == V4L2_FIELD_INTERLACED ||
+   fmt_src.format.field == V4L2_FIELD_INTERLACED_TB ||
+   fmt_src.format.field == V4L2_FIELD_INTERLACED_BT)
+   src_interlaced = true;
fmt_info = omap3isp_video_format_info(fmt_src.format.code);
depth_in = fmt_info-width;
}
@@ -1150,7 +1162,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 
omap3isp_configure_bridge(isp, ccdc-input, pdata, shift, bridge);
 
-   ccdc_config_sync_if(ccdc, pdata, depth_out);
+   ccdc_config_sync_if(ccdc, pdata, depth_out, src_interlaced);
 
syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 9584269..32d85c2 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -57,6 +57,8 @@ enum {
  * ISP_LANE_SHIFT_6 - CAMEXT[13:6] - CAM[7:0]
  * @clk_pol: Pixel clock polarity
  * 0 - Sample on rising edge, 1 - Sample on falling edge
+ * @fld_pol: Field identification signal polarity
+ * 0 - Active high, 1 - Active low
  * @hs_pol: Horizontal synchronization polarity
  * 0 - Active high, 1 - Active low
  * @vs_pol: Vertical synchronization polarity
@@ -67,6 +69,7 @@ enum {
 struct isp_parallel_platform_data {
unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
+   unsigned int fld_pol:1;
unsigned int hs_pol:1;
unsigned int vs_pol:1;
unsigned int data_pol:1;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] omap3isp: Add support for interlaced input data

2012-12-17 Thread William Swanson
If the remote video sensor reports an interlaced video mode, the CCDC block
should configure itself appropriately.
---
 drivers/media/platform/omap3isp/ispccdc.c |   16 ++--
 include/media/omap3isp.h  |3 +++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispccdc.c 
b/drivers/media/platform/omap3isp/ispccdc.c
index 60e60aa..5443ef4 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -970,10 +970,11 @@ void omap3isp_ccdc_max_rate(struct isp_ccdc_device *ccdc,
  * @ccdc: Pointer to ISP CCDC device.
  * @pdata: Parallel interface platform data (may be NULL)
  * @data_size: Data size
+ * @interlaced: Use interlaced mode instead of progressive mode
  */
 static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
struct isp_parallel_platform_data *pdata,
-   unsigned int data_size)
+   unsigned int data_size, bool interlaced)
 {
struct isp_device *isp = to_isp_device(ccdc);
const struct v4l2_mbus_framefmt *format;
@@ -1004,9 +1005,15 @@ static void ccdc_config_sync_if(struct isp_ccdc_device 
*ccdc,
break;
}
 
+   if (interlaced)
+   syn_mode |= ISPCCDC_SYN_MODE_FLDMODE;
+
if (pdata  pdata-data_pol)
syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
 
+   if (pdata  pdata-fld_pol)
+   syn_mode |= ISPCCDC_SYN_MODE_FLDPOL;
+
if (pdata  pdata-hs_pol)
syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
 
@@ -,6 +1118,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
const struct v4l2_rect *crop;
const struct isp_format_info *fmt_info;
struct v4l2_subdev_format fmt_src;
+   bool src_interlaced = false;
unsigned int depth_out;
unsigned int depth_in = 0;
struct media_pad *pad;
@@ -1132,6 +1140,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
fmt_src.pad = pad-index;
fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, fmt_src)) {
+   if (fmt_src.format.field == V4L2_FIELD_INTERLACED ||
+   fmt_src.format.field == V4L2_FIELD_INTERLACED_TB ||
+   fmt_src.format.field == V4L2_FIELD_INTERLACED_BT)
+   src_interlaced = true;
fmt_info = omap3isp_video_format_info(fmt_src.format.code);
depth_in = fmt_info-width;
}
@@ -1150,7 +1162,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 
omap3isp_configure_bridge(isp, ccdc-input, pdata, shift, bridge);
 
-   ccdc_config_sync_if(ccdc, pdata, depth_out);
+   ccdc_config_sync_if(ccdc, pdata, depth_out, src_interlaced);
 
syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 9584269..32d85c2 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -57,6 +57,8 @@ enum {
  * ISP_LANE_SHIFT_6 - CAMEXT[13:6] - CAM[7:0]
  * @clk_pol: Pixel clock polarity
  * 0 - Sample on rising edge, 1 - Sample on falling edge
+ * @fld_pol: Field identification signal polarity
+ * 0 - Active high, 1 - Active low
  * @hs_pol: Horizontal synchronization polarity
  * 0 - Active high, 1 - Active low
  * @vs_pol: Vertical synchronization polarity
@@ -67,6 +69,7 @@ enum {
 struct isp_parallel_platform_data {
unsigned int data_lane_shift:2;
unsigned int clk_pol:1;
+   unsigned int fld_pol:1;
unsigned int hs_pol:1;
unsigned int vs_pol:1;
unsigned int data_pol:1;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html