All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH - v1 6/6] DaVinci - Adding support for vpfe capture on DM365
@ 2009-12-10 17:00 m-karicheri2
  2009-12-10 17:00 ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files m-karicheri2
  0 siblings, 1 reply; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

Adding platform code for supporting vpfe capture and ISIF driver on DM365.

Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
 arch/arm/mach-davinci/board-dm365-evm.c    |   71 +++++++++++++++++++
 arch/arm/mach-davinci/dm365.c              |  106 +++++++++++++++++++++++++++-
 arch/arm/mach-davinci/include/mach/dm365.h |    2 +
 3 files changed, 178 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-davinci/board-dm365-evm.c b/arch/arm/mach-davinci/board-dm365-evm.c
index 8d23972..267898f 100644
--- a/arch/arm/mach-davinci/board-dm365-evm.c
+++ b/arch/arm/mach-davinci/board-dm365-evm.c
@@ -37,6 +37,8 @@
 #include <mach/nand.h>
 #include <mach/keyscan.h>
 
+#include <media/tvp514x.h>
+
 static inline int have_imager(void)
 {
 	/* REVISIT when it's supported, trigger via Kconfig */
@@ -305,6 +307,73 @@ static void dm365evm_mmc_configure(void)
 	davinci_cfg_reg(DM365_SD1_DATA0);
 }
 
+static struct tvp514x_platform_data tvp5146_pdata = {
+	.clk_polarity = 0,
+	.hs_polarity = 1,
+	.vs_polarity = 1
+};
+
+#define TVP514X_STD_ALL        (V4L2_STD_NTSC | V4L2_STD_PAL)
+/* Inputs available at the TVP5146 */
+static struct v4l2_input tvp5146_inputs[] = {
+	{
+		.index = 0,
+		.name = "Composite",
+		.type = V4L2_INPUT_TYPE_CAMERA,
+		.std = TVP514X_STD_ALL,
+	},
+	{
+		.index = 1,
+		.name = "S-Video",
+		.type = V4L2_INPUT_TYPE_CAMERA,
+		.std = TVP514X_STD_ALL,
+	},
+};
+
+/*
+ * this is the route info for connecting each input to decoder
+ * ouput that goes to vpfe. There is a one to one correspondence
+ * with tvp5146_inputs
+ */
+static struct vpfe_route tvp5146_routes[] = {
+	{
+		.input = INPUT_CVBS_VI2B,
+		.output = OUTPUT_10BIT_422_EMBEDDED_SYNC,
+	},
+{
+		.input = INPUT_SVIDEO_VI2C_VI1C,
+		.output = OUTPUT_10BIT_422_EMBEDDED_SYNC,
+	},
+};
+
+static struct vpfe_subdev_info vpfe_sub_devs[] = {
+	{
+		.name = "tvp5146",
+		.grp_id = 0,
+		.num_inputs = ARRAY_SIZE(tvp5146_inputs),
+		.inputs = tvp5146_inputs,
+		.routes = tvp5146_routes,
+		.can_route = 1,
+		.ccdc_if_params = {
+			.if_type = VPFE_BT656,
+			.hdpol = VPFE_PINPOL_POSITIVE,
+			.vdpol = VPFE_PINPOL_POSITIVE,
+		},
+		.board_info = {
+			I2C_BOARD_INFO("tvp5146", 0x5d),
+			.platform_data = &tvp5146_pdata,
+		},
+	},
+};
+
+static struct vpfe_config vpfe_cfg = {
+       .num_subdevs = ARRAY_SIZE(vpfe_sub_devs),
+       .sub_devs = vpfe_sub_devs,
+	.i2c_adapter_id = 1,
+       .card_name = "DM365 EVM",
+       .ccdc = "ISIF",
+};
+
 static void __init evm_init_i2c(void)
 {
 	davinci_init_i2c(&i2c_pdata);
@@ -496,6 +565,8 @@ static struct davinci_uart_config uart_config __initdata = {
 
 static void __init dm365_evm_map_io(void)
 {
+	/* setup input configuration for VPFE input devices */
+	dm365_set_vpfe_config(&vpfe_cfg);
 	dm365_init();
 }
 
diff --git a/arch/arm/mach-davinci/dm365.c b/arch/arm/mach-davinci/dm365.c
index cc3bae4..96eb83d 100644
--- a/arch/arm/mach-davinci/dm365.c
+++ b/arch/arm/mach-davinci/dm365.c
@@ -403,6 +403,11 @@ static struct clk mjcp_clk = {
 	.lpsc		= DM365_LPSC_MJCP,
 };
 
+static struct clk isif_clk = {
+	.name		= "isif",
+	.parent		= &vpss_master_clk,
+};
+
 static struct davinci_clk dm365_clks[] = {
 	CLK(NULL, "ref", &ref_clk),
 	CLK(NULL, "pll1", &pll1_clk),
@@ -459,6 +464,7 @@ static struct davinci_clk dm365_clks[] = {
 	CLK("davinci-asp.0", NULL, &asp0_clk),
 	CLK(NULL, "rto", &rto_clk),
 	CLK(NULL, "mjcp", &mjcp_clk),
+	CLK("isif", "master", &isif_clk),
 	CLK(NULL, NULL, NULL),
 };
 
@@ -1009,6 +1015,97 @@ void __init dm365_init(void)
 	davinci_common_init(&davinci_soc_info_dm365);
 }
 
+static struct resource dm365_vpss_resources[] = {
+	{
+		/* VPSS ISP5 Base address */
+		.name           = "isp5",
+		.start          = 0x01c70000,
+		.end            = 0x01c70000 + 0xff,
+		.flags          = IORESOURCE_MEM,
+	},
+	{
+		/* VPSS CLK Base address */
+		.name           = "vpss",
+		.start          = 0x01c70200,
+		.end            = 0x01c70200 + 0xff,
+		.flags          = IORESOURCE_MEM,
+	},
+};
+
+static struct platform_device dm365_vpss_device = {
+       .name                   = "vpss",
+       .id                     = -1,
+       .dev.platform_data      = "dm365_vpss",
+       .num_resources          = ARRAY_SIZE(dm365_vpss_resources),
+       .resource               = dm365_vpss_resources,
+};
+
+static struct resource vpfe_resources[] = {
+	{
+		.start          = IRQ_VDINT0,
+		.end            = IRQ_VDINT0,
+		.flags          = IORESOURCE_IRQ,
+	},
+	{
+		.start          = IRQ_VDINT1,
+		.end            = IRQ_VDINT1,
+		.flags          = IORESOURCE_IRQ,
+	},
+};
+
+static u64 vpfe_capture_dma_mask = DMA_BIT_MASK(32);
+static struct platform_device vpfe_capture_dev = {
+	.name           = CAPTURE_DRV_NAME,
+	.id             = -1,
+	.num_resources  = ARRAY_SIZE(vpfe_resources),
+	.resource       = vpfe_resources,
+	.dev = {
+		.dma_mask               = &vpfe_capture_dma_mask,
+		.coherent_dma_mask      = DMA_BIT_MASK(32),
+	},
+};
+
+static void dm365_isif_setup_pinmux(void)
+{
+	davinci_cfg_reg(DM365_VIN_CAM_WEN);
+	davinci_cfg_reg(DM365_VIN_CAM_VD);
+	davinci_cfg_reg(DM365_VIN_CAM_HD);
+	davinci_cfg_reg(DM365_VIN_YIN4_7_EN);
+	davinci_cfg_reg(DM365_VIN_YIN0_3_EN);
+}
+
+static struct resource isif_resource[] = {
+	/* ISIF Base address */
+	{
+		.start          = 0x01c71000,
+		.end            = 0x01c71000 + 0x1ff,
+		.flags          = IORESOURCE_MEM,
+	},
+	/* ISIF Linearization table 0 */
+	{
+		.start          = 0x1C7C000,
+		.end            = 0x1C7C000 + 0x2ff,
+		.flags          = IORESOURCE_MEM,
+	},
+	/* ISIF Linearization table 1 */
+	{
+		.start          = 0x1C7C400,
+		.end            = 0x1C7C400 + 0x2ff,
+		.flags          = IORESOURCE_MEM,
+	},
+};
+static struct platform_device dm365_isif_dev = {
+	.name           = "isif",
+	.id             = -1,
+	.num_resources  = ARRAY_SIZE(isif_resource),
+	.resource       = isif_resource,
+	.dev = {
+		.dma_mask               = &vpfe_capture_dma_mask,
+		.coherent_dma_mask      = DMA_BIT_MASK(32),
+		.platform_data		= dm365_isif_setup_pinmux,
+	},
+};
+
 static int __init dm365_init_devices(void)
 {
 	if (!cpu_is_davinci_dm365())
@@ -1017,7 +1114,14 @@ static int __init dm365_init_devices(void)
 	davinci_cfg_reg(DM365_INT_EDMA_CC);
 	platform_device_register(&dm365_edma_device);
 	platform_device_register(&dm365_emac_device);
-
+	platform_device_register(&dm365_vpss_device);
+	platform_device_register(&dm365_isif_dev);
+	platform_device_register(&vpfe_capture_dev);
 	return 0;
 }
 postcore_initcall(dm365_init_devices);
+
+void dm365_set_vpfe_config(struct vpfe_config *cfg)
+{
+       vpfe_capture_dev.dev.platform_data = cfg;
+}
diff --git a/arch/arm/mach-davinci/include/mach/dm365.h b/arch/arm/mach-davinci/include/mach/dm365.h
index 3c07a88..9b40f42 100644
--- a/arch/arm/mach-davinci/include/mach/dm365.h
+++ b/arch/arm/mach-davinci/include/mach/dm365.h
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/davinci_emac.h>
 #include <mach/hardware.h>
+#include <media/davinci/vpfe_capture.h>
 #include <mach/asp.h>
 #include <mach/keyscan.h>
 
@@ -36,4 +37,5 @@ void __init dm365_init_asp(struct snd_platform_data *pdata);
 void __init dm365_init_ks(struct davinci_ks_platform_data *pdata);
 void __init dm365_init_rtc(void);
 
+void dm365_set_vpfe_config(struct vpfe_config *cfg);
 #endif /* __ASM_ARCH_DM365_H */
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files
  2009-12-10 17:00 [PATCH - v1 6/6] DaVinci - Adding support for vpfe capture on DM365 m-karicheri2
@ 2009-12-10 17:00 ` m-karicheri2
  2009-12-10 17:00   ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source m-karicheri2
  2009-12-15  7:46   ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files Hans Verkuil
  0 siblings, 2 replies; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

This is the header file for ISIF driver on DM365. This has comments incorporated from
initial version.

ISIF driver is equivalent to CCDC driver on DM355 and DM644x. This driver is tested for
YUV capture from TVP514x driver. This patch contains the header files required for
this driver. The name of the file is changed to reflect the name of IP.

Reviewed-by: Nori, Sekhar <nsekhar@ti.com>
Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
Applies to linux-next tree of v4l-dvb 
 drivers/media/video/davinci/isif_regs.h |  290 ++++++++++++++++
 include/media/davinci/isif.h            |  554 +++++++++++++++++++++++++++++++
 2 files changed, 844 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/davinci/isif_regs.h
 create mode 100644 include/media/davinci/isif.h

diff --git a/drivers/media/video/davinci/isif_regs.h b/drivers/media/video/davinci/isif_regs.h
new file mode 100644
index 0000000..8e4c556
--- /dev/null
+++ b/drivers/media/video/davinci/isif_regs.h
@@ -0,0 +1,290 @@
+#ifndef _ISIF_REGS_H
+#define _ISIF_REGS_H
+
+/* ISIF registers relative offsets */
+#define SYNCEN					0x00
+#define MODESET					0x04
+#define HDW					0x08
+#define VDW					0x0c
+#define PPLN					0x10
+#define LPFR					0x14
+#define SPH					0x18
+#define LNH					0x1c
+#define SLV0					0x20
+#define SLV1					0x24
+#define LNV					0x28
+#define CULH					0x2c
+#define CULV					0x30
+#define HSIZE					0x34
+#define SDOFST					0x38
+#define CADU					0x3c
+#define CADL					0x40
+#define LINCFG0					0x44
+#define LINCFG1					0x48
+#define CCOLP					0x4c
+#define CRGAIN 					0x50
+#define CGRGAIN					0x54
+#define CGBGAIN					0x58
+#define CBGAIN					0x5c
+#define COFSTA					0x60
+#define FLSHCFG0				0x64
+#define FLSHCFG1				0x68
+#define FLSHCFG2				0x6c
+#define VDINT0					0x70
+#define VDINT1					0x74
+#define VDINT2					0x78
+#define MISC 					0x7c
+#define CGAMMAWD				0x80
+#define REC656IF				0x84
+#define CCDCFG					0x88
+/*****************************************************
+* Defect Correction registers
+*****************************************************/
+#define DFCCTL					0x8c
+#define VDFSATLV				0x90
+#define DFCMEMCTL				0x94
+#define DFCMEM0					0x98
+#define DFCMEM1					0x9c
+#define DFCMEM2					0xa0
+#define DFCMEM3					0xa4
+#define DFCMEM4					0xa8
+/****************************************************
+* Black Clamp registers
+****************************************************/
+#define CLAMPCFG				0xac
+#define CLDCOFST				0xb0
+#define CLSV					0xb4
+#define CLHWIN0					0xb8
+#define CLHWIN1					0xbc
+#define CLHWIN2					0xc0
+#define CLVRV					0xc4
+#define CLVWIN0					0xc8
+#define CLVWIN1					0xcc
+#define CLVWIN2					0xd0
+#define CLVWIN3					0xd4
+/****************************************************
+* Lense Shading Correction
+****************************************************/
+#define DATAHOFST				0xd8
+#define DATAVOFST				0xdc
+#define LSCHVAL					0xe0
+#define LSCVVAL					0xe4
+#define TWODLSCCFG				0xe8
+#define TWODLSCOFST				0xec
+#define TWODLSCINI				0xf0
+#define TWODLSCGRBU				0xf4
+#define TWODLSCGRBL				0xf8
+#define TWODLSCGROF				0xfc
+#define TWODLSCORBU				0x100
+#define TWODLSCORBL				0x104
+#define TWODLSCOROF				0x108
+#define TWODLSCIRQEN				0x10c
+#define TWODLSCIRQST				0x110
+/****************************************************
+* Data formatter
+****************************************************/
+#define FMTCFG					0x114
+#define FMTPLEN					0x118
+#define FMTSPH					0x11c
+#define FMTLNH					0x120
+#define FMTSLV					0x124
+#define FMTLNV					0x128
+#define FMTRLEN					0x12c
+#define FMTHCNT					0x130
+#define FMTAPTR_BASE				0x134
+/* Below macro for addresses FMTAPTR0 - FMTAPTR15 */
+#define FMTAPTR(i)			(FMTAPTR_BASE + (i * 4))
+#define FMTPGMVF0				0x174
+#define FMTPGMVF1				0x178
+#define FMTPGMAPU0				0x17c
+#define FMTPGMAPU1				0x180
+#define FMTPGMAPS0				0x184
+#define FMTPGMAPS1				0x188
+#define FMTPGMAPS2				0x18c
+#define FMTPGMAPS3				0x190
+#define FMTPGMAPS4				0x194
+#define FMTPGMAPS5				0x198
+#define FMTPGMAPS6				0x19c
+#define FMTPGMAPS7				0x1a0
+/************************************************
+* Color Space Converter
+************************************************/
+#define CSCCTL					0x1a4
+#define CSCM0					0x1a8
+#define CSCM1					0x1ac
+#define CSCM2					0x1b0
+#define CSCM3					0x1b4
+#define CSCM4					0x1b8
+#define CSCM5					0x1bc
+#define CSCM6					0x1c0
+#define CSCM7					0x1c4
+#define OBWIN0					0x1c8
+#define OBWIN1					0x1cc
+#define OBWIN2					0x1d0
+#define OBWIN3					0x1d4
+#define OBVAL0					0x1d8
+#define OBVAL1					0x1dc
+#define OBVAL2					0x1e0
+#define OBVAL3					0x1e4
+#define OBVAL4					0x1e8
+#define OBVAL5					0x1ec
+#define OBVAL6					0x1f0
+#define OBVAL7					0x1f4
+#define CLKCTL					0x1f8
+
+/* Masks & Shifts below */
+#define START_PX_HOR_MASK			(0x7FFF)
+#define NUM_PX_HOR_MASK				(0x7FFF)
+#define START_VER_ONE_MASK			(0x7FFF)
+#define START_VER_TWO_MASK			(0x7FFF)
+#define NUM_LINES_VER				(0x7FFF)
+
+/* gain - offset masks */
+#define GAIN_INTEGER_MASK			(0x7)
+#define GAIN_INTEGER_SHIFT			(0x9)
+#define GAIN_DECIMAL_MASK			(0x1FF)
+#define OFFSET_MASK			  	(0xFFF)
+#define GAIN_SDRAM_EN_SHIFT			(12)
+#define GAIN_IPIPE_EN_SHIFT			(13)
+#define GAIN_H3A_EN_SHIFT			(14)
+#define OFST_SDRAM_EN_SHIFT			(8)
+#define OFST_IPIPE_EN_SHIFT			(9)
+#define OFST_H3A_EN_SHIFT			(10)
+#define GAIN_OFFSET_EN_MASK			(0x7700)
+
+/* Culling */
+#define CULL_PAT_EVEN_LINE_SHIFT		(8)
+
+/* CCDCFG register */
+#define ISIF_YCINSWP_RAW			(0x00 << 4)
+#define ISIF_YCINSWP_YCBCR			(0x01 << 4)
+#define ISIF_CCDCFG_FIDMD_LATCH_VSYNC		(0x00 << 6)
+#define ISIF_CCDCFG_WENLOG_AND			(0x00 << 8)
+#define ISIF_CCDCFG_TRGSEL_WEN			(0x00 << 9)
+#define ISIF_CCDCFG_EXTRG_DISABLE		(0x00 << 10)
+#define ISIF_LATCH_ON_VSYNC_DISABLE		(0x01 << 15)
+#define ISIF_LATCH_ON_VSYNC_ENABLE		(0x00 << 15)
+#define ISIF_DATA_PACK_MASK			(0x03)
+#define ISIF_DATA_PACK16			(0x0)
+#define ISIF_DATA_PACK12			(0x1)
+#define ISIF_DATA_PACK8				(0x2)
+#define ISIF_PIX_ORDER_SHIFT			(11)
+#define ISIF_PIX_ORDER_MASK			(0x01)
+#define ISIF_BW656_ENABLE			(0x01 << 5)
+
+/* MODESET registers */
+#define ISIF_VDHDOUT_INPUT			(0x00 << 0)
+#define ISIF_INPUT_MASK				(0x03)
+#define ISIF_INPUT_SHIFT			(12)
+#define ISIF_RAW_INPUT_MODE			(0x00)
+#define ISIF_FID_POL_MASK			(0x01)
+#define ISIF_FID_POL_SHIFT			(4)
+#define ISIF_HD_POL_MASK			(0x01)
+#define ISIF_HD_POL_SHIFT			(3)
+#define ISIF_VD_POL_MASK			(0x01)
+#define ISIF_VD_POL_SHIFT			(2)
+#define ISIF_DATAPOL_NORMAL			(0x00)
+#define ISIF_DATAPOL_MASK			(0x01)
+#define ISIF_DATAPOL_SHIFT			(6)
+#define ISIF_EXWEN_DISABLE 			(0x00)
+#define ISIF_EXWEN_MASK				(0x01)
+#define ISIF_EXWEN_SHIFT			(5)
+#define ISIF_FRM_FMT_MASK			(0x01)
+#define ISIF_FRM_FMT_SHIFT			(7)
+#define ISIF_DATASFT_MASK			(0x07)
+#define ISIF_DATASFT_SHIFT			(8)
+#define ISIF_LPF_SHIFT				(14)
+#define ISIF_LPF_MASK				(0x1)
+
+/* GAMMAWD registers */
+#define ISIF_ALAW_GAMA_WD_MASK			(0xF)
+#define ISIF_ALAW_GAMA_WD_SHIFT			(1)
+#define ISIF_ALAW_ENABLE			(0x01)
+#define ISIF_GAMMAWD_CFA_MASK			(0x01)
+#define ISIF_GAMMAWD_CFA_SHIFT			(5)
+
+/* HSIZE registers */
+#define ISIF_HSIZE_FLIP_MASK			(0x01)
+#define ISIF_HSIZE_FLIP_SHIFT			(12)
+#define ISIF_LINEOFST_MASK			(0xFFF)
+
+/* MISC registers */
+#define ISIF_DPCM_EN_SHIFT			(12)
+#define ISIF_DPCM_EN_MASK			(1)
+#define ISIF_DPCM_PREDICTOR_SHIFT		(13)
+#define ISIF_DPCM_PREDICTOR_MASK 		(1)
+
+/* Black clamp related */
+#define ISIF_BC_DCOFFSET_MASK			(0x1FFF)
+#define ISIF_BC_MODE_COLOR_MASK			(1)
+#define ISIF_BC_MODE_COLOR_SHIFT		(4)
+#define ISIF_HORZ_BC_MODE_MASK			(3)
+#define ISIF_HORZ_BC_MODE_SHIFT			(1)
+#define ISIF_HORZ_BC_WIN_COUNT_MASK		(0x1F)
+#define ISIF_HORZ_BC_WIN_SEL_SHIFT		(5)
+#define ISIF_HORZ_BC_PIX_LIMIT_SHIFT		(6)
+#define ISIF_HORZ_BC_WIN_H_SIZE_MASK		(3)
+#define ISIF_HORZ_BC_WIN_H_SIZE_SHIFT		(8)
+#define ISIF_HORZ_BC_WIN_V_SIZE_MASK		(3)
+#define ISIF_HORZ_BC_WIN_V_SIZE_SHIFT		(12)
+#define ISIF_HORZ_BC_WIN_START_H_MASK		(0x1FFF)
+#define ISIF_HORZ_BC_WIN_START_V_MASK		(0x1FFF)
+#define ISIF_VERT_BC_OB_H_SZ_MASK		(7)
+#define ISIF_VERT_BC_RST_VAL_SEL_MASK		(3)
+#define	ISIF_VERT_BC_RST_VAL_SEL_SHIFT		(4)
+#define ISIF_VERT_BC_LINE_AVE_COEF_SHIFT	(8)
+#define	ISIF_VERT_BC_OB_START_HORZ_MASK		(0x1FFF)
+#define ISIF_VERT_BC_OB_START_VERT_MASK		(0x1FFF)
+#define ISIF_VERT_BC_OB_VERT_SZ_MASK		(0x1FFF)
+#define ISIF_VERT_BC_RST_VAL_MASK		(0xFFF)
+#define ISIF_BC_VERT_START_SUB_V_MASK		(0x1FFF)
+
+/* VDFC registers */
+#define ISIF_VDFC_EN_SHIFT			(4)
+#define ISIF_VDFC_CORR_MOD_MASK			(3)
+#define ISIF_VDFC_CORR_MOD_SHIFT		(5)
+#define ISIF_VDFC_CORR_WHOLE_LN_SHIFT		(7)
+#define ISIF_VDFC_LEVEL_SHFT_MASK		(7)
+#define ISIF_VDFC_LEVEL_SHFT_SHIFT		(8)
+#define ISIF_VDFC_SAT_LEVEL_MASK		(0xFFF)
+#define ISIF_VDFC_POS_MASK			(0x1FFF)
+#define ISIF_DFCMEMCTL_DFCMARST_SHIFT		(2)
+
+/* CSC registers */
+#define ISIF_CSC_COEF_INTEG_MASK		(7)
+#define ISIF_CSC_COEF_DECIMAL_MASK		(0x1f)
+#define ISIF_CSC_COEF_INTEG_SHIFT		(5)
+#define ISIF_CSCM_MSB_SHIFT			(8)
+#define ISIF_DF_CSC_SPH_MASK			(0x1FFF)
+#define ISIF_DF_CSC_LNH_MASK			(0x1FFF)
+#define ISIF_DF_CSC_SLV_MASK			(0x1FFF)
+#define ISIF_DF_CSC_LNV_MASK			(0x1FFF)
+#define ISIF_DF_NUMLINES			(0x7FFF)
+#define ISIF_DF_NUMPIX				(0x1FFF)
+
+/* Offsets for LSC/DFC/Gain */
+#define ISIF_DATA_H_OFFSET_MASK			(0x1FFF)
+#define ISIF_DATA_V_OFFSET_MASK			(0x1FFF)
+
+/* Linearization */
+#define ISIF_LIN_CORRSFT_MASK			(7)
+#define ISIF_LIN_CORRSFT_SHIFT			(4)
+#define ISIF_LIN_SCALE_FACT_INTEG_SHIFT		(10)
+#define ISIF_LIN_SCALE_FACT_DECIMAL_MASK	(0x3FF)
+#define ISIF_LIN_ENTRY_MASK			(0x3FF)
+
+#define ISIF_DF_FMTRLEN_MASK			(0x1FFF)
+#define ISIF_DF_FMTHCNT_MASK			(0x1FFF)
+
+/* Pattern registers */
+#define ISIF_PG_EN				(1 << 3)
+#define ISIF_SEL_PG_SRC				(3 << 4)
+#define ISIF_PG_VD_POL_SHIFT			(0)
+#define ISIF_PG_HD_POL_SHIFT			(1)
+
+/*random other junk*/
+#define ISIF_SYNCEN_VDHDEN_MASK			(1 << 0)
+#define ISIF_SYNCEN_WEN_MASK			(1 << 1)
+#define ISIF_SYNCEN_WEN_SHIFT			1
+
+#endif
diff --git a/include/media/davinci/isif.h b/include/media/davinci/isif.h
new file mode 100644
index 0000000..b75fea7
--- /dev/null
+++ b/include/media/davinci/isif.h
@@ -0,0 +1,554 @@
+/*
+ * Copyright (C) 2008-2009 Texas Instruments Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * isif header file
+ */
+#ifndef _ISIF_H
+#define _ISIF_H
+#include <media/davinci/ccdc_types.h>
+#include <media/davinci/vpfe_types.h>
+/* isif float type S8Q8/U8Q8 */
+struct isif_float_8 {
+	/* 8 bit integer part */
+	__u8 integer;
+	/* 8 bit decimal part */
+	__u8 decimal;
+};
+
+/* isif float type U16Q16/S16Q16 */
+struct isif_float_16 {
+	/* 16 bit integer part */
+	__u16 integer;
+	/* 16 bit decimal part */
+	__u16 decimal;
+};
+
+/************************************************************************
+ *   Vertical Defect Correction parameters
+ ***********************************************************************/
+/* Defect Correction (DFC) table entry */
+struct isif_vdfc_entry {
+	/* vertical position of defect */
+	__u16 pos_vert;
+	/* horizontal position of defect */
+	__u16 pos_horz;
+	/*
+	 * Defect level of Vertical line defect position. This is subtracted
+	 * from the data at the defect position
+	 */
+	__u8 level_at_pos;
+	/*
+	 * Defect level of the pixels upper than the vertical line defect.
+	 * This is subtracted from the data
+	 */
+	__u8 level_up_pixels;
+	/*
+	 * Defect level of the pixels lower than the vertical line defect.
+	 * This is subtracted from the data
+	 */
+	__u8 level_low_pixels;
+};
+
+#define ISIF_VDFC_TABLE_SIZE		8
+struct isif_dfc {
+	/* enable vertical defect correction */
+	__u8 en;
+	/* Defect level subtraction. Just fed through if saturating */
+#define	ISIF_VDFC_NORMAL		0
+	/*
+	 * Defect level subtraction. Horizontal interpolation ((i-2)+(i+2))/2
+	 * if data saturating
+	 */
+#define ISIF_VDFC_HORZ_INTERPOL_IF_SAT	1
+	/* Horizontal interpolation (((i-2)+(i+2))/2) */
+#define	ISIF_VDFC_HORZ_INTERPOL		2
+	/* one of the vertical defect correction modes above */
+	__u8 corr_mode;
+	/* 0 - whole line corrected, 1 - not pixels upper than the defect */
+	__u8 corr_whole_line;
+#define ISIF_VDFC_NO_SHIFT		0
+#define ISIF_VDFC_SHIFT_1		1
+#define ISIF_VDFC_SHIFT_2		2
+#define ISIF_VDFC_SHIFT_3		3
+#define ISIF_VDFC_SHIFT_4		4
+	/*
+	 * defect level shift value. level_at_pos, level_upper_pos,
+	 * and level_lower_pos can be shifted up by this value. Choose
+	 * one of the values above
+	 */
+	__u8 def_level_shift;
+	/* defect saturation level */
+	__u16 def_sat_level;
+	/* number of vertical defects. Max is ISIF_VDFC_TABLE_SIZE */
+	__u16 num_vdefects;
+	/* VDFC table ptr */
+	struct isif_vdfc_entry table[ISIF_VDFC_TABLE_SIZE];
+};
+
+struct isif_horz_bclamp {
+
+	/* Horizontal clamp disabled. Only vertical clamp value is subtracted */
+#define	ISIF_HORZ_BC_DISABLE		0
+	/*
+	 * Horizontal clamp value is calculated and subtracted from image data
+	 * along with vertical clamp value
+	 */
+#define ISIF_HORZ_BC_CLAMP_CALC_ENABLED	1
+	/*
+	 * Horizontal clamp value calculated from previous image is subtracted
+	 * from image data along with vertical clamp value.
+	 */
+#define ISIF_HORZ_BC_CLAMP_NOT_UPDATED	2
+	/* horizontal clamp mode. One of the values above */
+	__u8 mode;
+	/*
+	 * pixel value limit enable.
+	 *  0 - limit disabled
+	 *  1 - pixel value limited to 1023
+	 */
+	__u8 clamp_pix_limit;
+	/* Select Most left window for bc calculation */
+#define	ISIF_SEL_MOST_LEFT_WIN		0
+	/* Select Most right window for bc calculation */
+#define ISIF_SEL_MOST_RIGHT_WIN		1
+	/* Select most left or right window for clamp val calculation */
+	__u8 base_win_sel_calc;
+	/* Window count per color for calculation. range 1-32 */
+	__u8 win_count_calc;
+	/* Window start position - horizontal for calculation. 0 - 8191 */
+	__u16 win_start_h_calc;
+	/* Window start position - vertical for calculation 0 - 8191 */
+	__u16 win_start_v_calc;
+#define ISIF_HORZ_BC_SZ_H_2PIXELS	0
+#define ISIF_HORZ_BC_SZ_H_4PIXELS	1
+#define ISIF_HORZ_BC_SZ_H_8PIXELS	2
+#define ISIF_HORZ_BC_SZ_H_16PIXELS	3
+	/* Width of the sample window in pixels for calculation */
+	__u8 win_h_sz_calc;
+#define ISIF_HORZ_BC_SZ_V_32PIXELS	0
+#define ISIF_HORZ_BC_SZ_V_64PIXELS	1
+#define	ISIF_HORZ_BC_SZ_V_128PIXELS	2
+#define ISIF_HORZ_BC_SZ_V_256PIXELS	3
+	/* Height of the sample window in pixels for calculation */
+	__u8 win_v_sz_calc;
+};
+
+/************************************************************************
+ *  Black Clamp parameters
+ ***********************************************************************/
+struct isif_vert_bclamp {
+	/* Reset value used is the clamp value calculated */
+#define	ISIF_VERT_BC_USE_HORZ_CLAMP_VAL		0
+	/* Reset value used is reset_clamp_val configured */
+#define	ISIF_VERT_BC_USE_CONFIG_CLAMP_VAL	1
+	/* No update, previous image value is used */
+#define	ISIF_VERT_BC_NO_UPDATE			2
+	/*
+	 * Reset value selector for vertical clamp calculation. Use one of
+	 * the above values
+	 */
+	__u8 reset_val_sel;
+	/* U12 value if reset_val_sel = ISIF_BC_VERT_USE_CONFIG_CLAMP_VAL */
+	__u16 reset_clamp_val;
+	/* U8Q8. Line average coefficient used in vertical clamp calculation */
+	__u8 line_ave_coef;
+#define	ISIF_VERT_BC_SZ_H_2PIXELS	0
+#define ISIF_VERT_BC_SZ_H_4PIXELS	1
+#define	ISIF_VERT_BC_SZ_H_8PIXELS	2
+#define	ISIF_VERT_BC_SZ_H_16PIXELS	3
+#define	ISIF_VERT_BC_SZ_H_32PIXELS	4
+#define	ISIF_VERT_BC_SZ_H_64PIXELS	5
+	/* Width in pixels of the optical black region used for calculation */
+	__u8 ob_h_sz_calc;
+	/* Height of the optical black region for calculation */
+	__u16 ob_v_sz_calc;
+	/* Optical black region start position - horizontal. 0 - 8191 */
+	__u16 ob_start_h;
+	/* Optical black region start position - vertical 0 - 8191 */
+	__u16 ob_start_v;
+};
+
+struct isif_black_clamp {
+	/*
+	 * This offset value is added irrespective of the clamp enable status.
+	 * S13
+	 */
+	__u16 dc_offset;
+	/*
+	 * Enable black/digital clamp value to be subtracted from the image data
+	 */
+	__u8 en;
+	/*
+	 * black clamp mode. same/separate clamp for 4 colors
+	 * 0 - disable - same clamp value for all colors
+	 * 1 - clamp value calculated separately for all colors
+	 */
+	__u8 bc_mode_color;
+	/* Vrtical start position for bc subtraction */
+	__u16 vert_start_sub;
+	/* Black clamp for horizontal direction */
+	struct isif_horz_bclamp horz;
+	/* Black clamp for vertical direction */
+	struct isif_vert_bclamp vert;
+};
+
+/*************************************************************************
+** Color Space Convertion (CSC)
+*************************************************************************/
+#define ISIF_CSC_NUM_COEFF	16
+struct isif_color_space_conv {
+	/* Enable color space conversion */
+	__u8 en;
+	/*
+	 * csc coeffient table. S8Q5, M00 at index 0, M01 at index 1, and
+	 * so forth
+	 */
+	struct isif_float_8 coeff[ISIF_CSC_NUM_COEFF];
+};
+
+
+/*************************************************************************
+**  Black  Compensation parameters
+*************************************************************************/
+struct isif_black_comp {
+	/* Comp for Red */
+	__s8 r_comp;
+	/* Comp for Gr */
+	__s8 gr_comp;
+	/* Comp for Blue */
+	__s8 b_comp;
+	/* Comp for Gb */
+	__s8 gb_comp;
+};
+
+/*************************************************************************
+**  Gain parameters
+*************************************************************************/
+struct isif_gain {
+	/* Gain for Red or ye */
+	struct isif_float_16 r_ye;
+	/* Gain for Gr or cy */
+	struct isif_float_16 gr_cy;
+	/* Gain for Gb or g */
+	struct isif_float_16 gb_g;
+	/* Gain for Blue or mg */
+	struct isif_float_16 b_mg;
+};
+
+#define ISIF_LINEAR_TAB_SIZE	192
+/*************************************************************************
+**  Linearization parameters
+*************************************************************************/
+struct isif_linearize {
+	/* Enable or Disable linearization of data */
+	__u8 en;
+	/* Shift value applied */
+	__u8 corr_shft;
+	/* scale factor applied U11Q10 */
+	struct isif_float_16 scale_fact;
+	/* Size of the linear table */
+	__u16 table[ISIF_LINEAR_TAB_SIZE];
+};
+
+/* Color patterns */
+#define ISIF_RED	0
+#define	ISIF_GREEN_RED	1
+#define ISIF_GREEN_BLUE	2
+#define ISIF_BLUE	3
+struct isif_col_pat {
+	__u8 olop;
+	__u8 olep;
+	__u8 elop;
+	__u8 elep;
+};
+
+/*************************************************************************
+**  Data formatter parameters
+*************************************************************************/
+struct isif_fmtplen {
+	/*
+	 * number of program entries for SET0, range 1 - 16
+	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
+	 * ISIF_COMBINE
+	 */
+	__u16 plen0;
+	/*
+	 * number of program entries for SET1, range 1 - 16
+	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
+	 * ISIF_COMBINE
+	 */
+	__u16 plen1;
+	/**
+	 * number of program entries for SET2, range 1 - 16
+	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
+	 * ISIF_COMBINE
+	 */
+	__u16 plen2;
+	/**
+	 * number of program entries for SET3, range 1 - 16
+	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
+	 * ISIF_COMBINE
+	 */
+	__u16 plen3;
+};
+
+struct isif_fmt_cfg {
+#define ISIF_SPLIT		0
+#define ISIF_COMBINE		1
+	/* Split or combine or line alternate */
+	__u8 fmtmode;
+	/* enable or disable line alternating mode */
+	__u8 ln_alter_en;
+#define ISIF_1LINE		0
+#define	ISIF_2LINES		1
+#define	ISIF_3LINES		2
+#define	ISIF_4LINES		3
+	/* Split/combine line number */
+	__u8 lnum;
+	/* Address increment Range 1 - 16 */
+	__u8 addrinc;
+};
+
+struct isif_fmt_addr_ptr {
+	/* Initial address */
+	__u32 init_addr;
+	/* output line number */
+#define ISIF_1STLINE		0
+#define	ISIF_2NDLINE		1
+#define	ISIF_3RDLINE		2
+#define	ISIF_4THLINE		3
+	__u8 out_line;
+};
+
+struct isif_fmtpgm_ap {
+	/* program address pointer */
+	__u8 pgm_aptr;
+	/* program address increment or decrement */
+	__u8 pgmupdt;
+};
+
+struct isif_data_formatter {
+	/* Enable/Disable data formatter */
+	__u8 en;
+	/* data formatter configuration */
+	struct isif_fmt_cfg cfg;
+	/* Formatter program entries length */
+	struct isif_fmtplen plen;
+	/* first pixel in a line fed to formatter */
+	__u16 fmtrlen;
+	/* HD interval for output line. Only valid when split line */
+	__u16 fmthcnt;
+	/* formatter address pointers */
+	struct isif_fmt_addr_ptr fmtaddr_ptr[16];
+	/* program enable/disable */
+	__u8 pgm_en[32];
+	/* program address pointers */
+	struct isif_fmtpgm_ap fmtpgm_ap[32];
+};
+
+struct isif_df_csc {
+	/* Color Space Conversion confguration, 0 - csc, 1 - df */
+	__u8 df_or_csc;
+	/* csc configuration valid if df_or_csc is 0 */
+	struct isif_color_space_conv csc;
+	/* data formatter configuration valid if df_or_csc is 1 */
+	struct isif_data_formatter df;
+	/* start pixel in a line at the input */
+	__u32 start_pix;
+	/* number of pixels in input line */
+	__u32 num_pixels;
+	/* start line at the input */
+	__u32 start_line;
+	/* number of lines at the input */
+	__u32 num_lines;
+};
+
+struct isif_gain_offsets_adj {
+	/* Gain adjustment per color */
+	struct isif_gain gain;
+	/* Offset adjustment */
+	__u16 offset;
+	/* Enable or Disable Gain adjustment for SDRAM data */
+	__u8 gain_sdram_en;
+	/* Enable or Disable Gain adjustment for IPIPE data */
+	__u8 gain_ipipe_en;
+	/* Enable or Disable Gain adjustment for H3A data */
+	__u8 gain_h3a_en;
+	/* Enable or Disable Gain adjustment for SDRAM data */
+	__u8 offset_sdram_en;
+	/* Enable or Disable Gain adjustment for IPIPE data */
+	__u8 offset_ipipe_en;
+	/* Enable or Disable Gain adjustment for H3A data */
+	__u8 offset_h3a_en;
+};
+
+struct isif_cul {
+	/* Horizontal Cull pattern for odd lines */
+	__u8 hcpat_odd;
+	/* Horizontal Cull pattern for even lines */
+	__u8 hcpat_even;
+	/* Vertical Cull pattern */
+	__u8 vcpat;
+	/* Enable or disable lpf. Apply when cull is enabled */
+	__u8 en_lpf;
+};
+
+struct isif_compress {
+#define ISIF_ALAW		0
+#define ISIF_DPCM		1
+#define ISIF_NO_COMPRESSION	2
+	/* Compression Algorithm used */
+	__u8 alg;
+	/* Choose Predictor1 for DPCM compression */
+#define ISIF_DPCM_PRED1		0
+	/* Choose Predictor2 for DPCM compression */
+#define ISIF_DPCM_PRED2		1
+	/* Predictor for DPCM compression */
+	__u8 pred;
+};
+
+/* all the stuff in this struct will be provided by userland */
+struct isif_config_params_raw {
+	/* Linearization parameters for image sensor data input */
+	struct isif_linearize linearize;
+	/* Data formatter or CSC */
+	struct isif_df_csc df_csc;
+	/* Defect Pixel Correction (DFC) confguration */
+	struct isif_dfc dfc;
+	/* Black/Digital Clamp configuration */
+	struct isif_black_clamp bclamp;
+	/* Gain, offset adjustments */
+	struct isif_gain_offsets_adj gain_offset;
+	/* Culling */
+	struct isif_cul culling;
+	/* A-Law and DPCM compression options */
+	struct isif_compress compress;
+	/* horizontal offset for Gain/LSC/DFC */
+	__u16 horz_offset;
+	/* vertical offset for Gain/LSC/DFC */
+	__u16 vert_offset;
+	/* color pattern for field 0 */
+	struct isif_col_pat col_pat_field0;
+	/* color pattern for field 1 */
+	struct isif_col_pat col_pat_field1;
+	/* No Shift */
+#define ISIF_NO_SHIFT		0
+	/* 1 bit Shift */
+#define	ISIF_1BIT_SHIFT		1
+	/* 2 bit Shift */
+#define	ISIF_2BIT_SHIFT		2
+	/* 3 bit Shift */
+#define	ISIF_3BIT_SHIFT		3
+	/* 4 bit Shift */
+#define	ISIF_4BIT_SHIFT		4
+	/* 5 bit Shift */
+#define ISIF_5BIT_SHIFT		5
+	/* 6 bit Shift */
+#define ISIF_6BIT_SHIFT		6
+	/* Data shift applied before storing to SDRAM */
+	__u8 data_shift;
+	/* enable input test pattern generation */
+	__u8 test_pat_gen;
+};
+
+#ifdef __KERNEL__
+struct isif_ycbcr_config {
+	/* isif pixel format */
+	enum ccdc_pixfmt pix_fmt;
+	/* isif frame format */
+	enum ccdc_frmfmt frm_fmt;
+	/* ISIF crop window */
+	struct v4l2_rect win;
+	/* field polarity */
+	enum vpfe_pin_pol fid_pol;
+	/* interface VD polarity */
+	enum vpfe_pin_pol vd_pol;
+	/* interface HD polarity */
+	enum vpfe_pin_pol hd_pol;
+	/* isif pix order. Only used for ycbcr capture */
+	enum ccdc_pixorder pix_order;
+	/* isif buffer type. Only used for ycbcr capture */
+	enum ccdc_buftype buf_type;
+};
+
+/* MSB of image data connected to sensor port */
+enum isif_data_msb {
+	/* MSB b15 */
+	ISIF_BIT_MSB_15,
+	/* MSB b14 */
+	ISIF_BIT_MSB_14,
+	/* MSB b13 */
+	ISIF_BIT_MSB_13,
+	/* MSB b12 */
+	ISIF_BIT_MSB_12,
+	/* MSB b11 */
+	ISIF_BIT_MSB_11,
+	/* MSB b10 */
+	ISIF_BIT_MSB_10,
+	/* MSB b9 */
+	ISIF_BIT_MSB_9,
+	/* MSB b8 */
+	ISIF_BIT_MSB_8,
+	/* MSB b7 */
+	ISIF_BIT_MSB_7
+};
+
+enum isif_cfa_pattern {
+	ISIF_CFA_PAT_MOSAIC,
+	ISIF_CFA_PAT_STRIPE
+};
+
+struct isif_params_raw {
+	/* isif pixel format */
+	enum ccdc_pixfmt pix_fmt;
+	/* isif frame format */
+	enum ccdc_frmfmt frm_fmt;
+	/* video window */
+	struct v4l2_rect win;
+	/* field polarity */
+	enum vpfe_pin_pol fid_pol;
+	/* interface VD polarity */
+	enum vpfe_pin_pol vd_pol;
+	/* interface HD polarity */
+	enum vpfe_pin_pol hd_pol;
+	/* buffer type. Applicable for interlaced mode */
+	enum ccdc_buftype buf_type;
+	/* Gain values */
+	struct isif_gain gain;
+	/* cfa pattern */
+	enum isif_cfa_pattern cfa_pat;
+	/* Data MSB position */
+	enum isif_data_msb data_msb;
+	/* Enable horizontal flip */
+	unsigned char horz_flip_en;
+	/* Enable image invert vertically */
+	unsigned char image_invert_en;
+
+	/* all the userland defined stuff*/
+	struct isif_config_params_raw config_params;
+};
+
+enum isif_data_pack {
+	ISIF_PACK_16BIT,
+	ISIF_PACK_12BIT,
+	ISIF_PACK_8BIT
+};
+
+#define ISIF_WIN_NTSC				{0, 0, 720, 480}
+#define ISIF_WIN_VGA				{0, 0, 640, 480}
+#endif
+#endif
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source
  2009-12-10 17:00 ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files m-karicheri2
@ 2009-12-10 17:00   ` m-karicheri2
  2009-12-10 17:00     ` [PATCH - v1 1/6] V4L - vpfe-capture : DM365 vpss enhancements m-karicheri2
  2009-12-15  7:57     ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source Hans Verkuil
  2009-12-15  7:46   ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files Hans Verkuil
  1 sibling, 2 replies; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

This is the source file for ISIF driver for DM365. This has comments incorporated from
initial version.

ISIF driver is equivalent to CCDC driver on DM355 and DM644x. This driver is tested for
YUV capture from TVP514x driver. This patch contains the header files required for
this driver. The name of the file is changed to reflect the name of IP.

Reviewed-by: Nori, Sekhar <nsekhar@ti.com>
Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
Applies to linux-next tree of v4l-dvb 
 drivers/media/video/davinci/isif.c | 1498 ++++++++++++++++++++++++++++++++++++
 1 files changed, 1498 insertions(+), 0 deletions(-)
 create mode 100644 drivers/media/video/davinci/isif.c

diff --git a/drivers/media/video/davinci/isif.c b/drivers/media/video/davinci/isif.c
new file mode 100644
index 0000000..916afab
--- /dev/null
+++ b/drivers/media/video/davinci/isif.c
@@ -0,0 +1,1498 @@
+/*
+ * Copyright (C) 2008-2009 Texas Instruments Inc
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * This is the isif hardware module.
+ * TODO: 1) Raw bayer parameter settings and bayer capture
+ *	 2) Add support for control ioctl
+ */
+#include <linux/delay.h>
+#include <linux/platform_device.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include <linux/videodev2.h>
+#include <linux/clk.h>
+
+#include <mach/mux.h>
+
+#include <media/davinci/isif.h>
+#include <media/davinci/vpss.h>
+
+#include "isif_regs.h"
+#include "ccdc_hw_device.h"
+
+/* Defauts for module configuation paramaters */
+static struct isif_config_params_raw isif_config_defaults = {
+	.linearize = {
+		.en = 0,
+		.corr_shft = ISIF_NO_SHIFT,
+		.scale_fact = {1, 0},
+	},
+	.df_csc = {
+		.df_or_csc = 0,
+		.csc = {
+			.en = 0,
+		},
+	},
+	.dfc = {
+		.en = 0,
+	},
+	.bclamp = {
+		.en = 0,
+	},
+	.gain_offset = {
+		.gain = {
+			.r_ye = {1, 0},
+			.gr_cy = {1, 0},
+			.gb_g = {1, 0},
+			.b_mg = {1, 0},
+		},
+	},
+	.culling = {
+		.hcpat_odd = 0xff,
+		.hcpat_even = 0xff,
+		.vcpat = 0xff,
+	},
+	.compress = {
+		.alg = ISIF_ALAW,
+	},
+};
+
+/* ISIF operation configuration */
+static struct isif_oper_config {
+	struct device *dev;
+	enum vpfe_hw_if_type if_type;
+	struct isif_ycbcr_config ycbcr;
+	struct isif_params_raw bayer;
+	enum isif_data_pack data_pack;
+	/* Master clock */
+	struct clk *mclk;
+	/* ISIF base address */
+	void __iomem *base_addr;
+	/* ISIF Linear Table 0 */
+	void __iomem *linear_tbl0_addr;
+	/* ISIF Linear Table 1 */
+	void __iomem *linear_tbl1_addr;
+} isif_cfg = {
+	.ycbcr = {
+		.pix_fmt = CCDC_PIXFMT_YCBCR_8BIT,
+		.frm_fmt = CCDC_FRMFMT_INTERLACED,
+		.win = ISIF_WIN_NTSC,
+		.fid_pol = VPFE_PINPOL_POSITIVE,
+		.vd_pol = VPFE_PINPOL_POSITIVE,
+		.hd_pol = VPFE_PINPOL_POSITIVE,
+		.pix_order = CCDC_PIXORDER_CBYCRY,
+		.buf_type = CCDC_BUFTYPE_FLD_INTERLEAVED,
+	},
+	.bayer = {
+		.pix_fmt = CCDC_PIXFMT_RAW,
+		.frm_fmt = CCDC_FRMFMT_PROGRESSIVE,
+		.win = ISIF_WIN_VGA,
+		.fid_pol = VPFE_PINPOL_POSITIVE,
+		.vd_pol = VPFE_PINPOL_POSITIVE,
+		.hd_pol = VPFE_PINPOL_POSITIVE,
+		.gain = {
+			.r_ye = {1, 0},
+			.gr_cy = {1, 0},
+			.gb_g = {1, 0},
+			.b_mg = {1, 0},
+		},
+		.cfa_pat = ISIF_CFA_PAT_MOSAIC,
+		.data_msb = ISIF_BIT_MSB_11,
+		.config_params = {
+			.data_shift = ISIF_NO_SHIFT,
+			.col_pat_field0 = {
+				.olop = ISIF_GREEN_BLUE,
+				.olep = ISIF_BLUE,
+				.elop = ISIF_RED,
+				.elep = ISIF_GREEN_RED,
+			},
+			.col_pat_field1 = {
+				.olop = ISIF_GREEN_BLUE,
+				.olep = ISIF_BLUE,
+				.elop = ISIF_RED,
+				.elep = ISIF_GREEN_RED,
+			},
+			.test_pat_gen = 0,
+		},
+	},
+	.data_pack = ISIF_DATA_PACK8,
+};
+
+/* Raw Bayer formats */
+static u32 isif_raw_bayer_pix_formats[] =
+		{V4L2_PIX_FMT_SBGGR8, V4L2_PIX_FMT_SBGGR16};
+
+/* Raw YUV formats */
+static u32 isif_raw_yuv_pix_formats[] =
+		{V4L2_PIX_FMT_UYVY, V4L2_PIX_FMT_YUYV};
+
+/* register access routines */
+static inline u32 regr(u32 offset)
+{
+	return __raw_readl(isif_cfg.base_addr + offset);
+}
+
+static inline void regw(u32 val, u32 offset)
+{
+	__raw_writel(val, isif_cfg.base_addr + offset);
+}
+
+/* reg_modify() - read, modify and write register */
+static inline u32 reg_modify(u32 mask, u32 val, u32 offset)
+{
+	u32 new_val = (regr(offset) & ~mask) | (val & mask);
+
+	regw(new_val, offset);
+	return new_val;
+}
+
+static inline void regw_lin_tbl(u32 val, u32 offset, int i)
+{
+	if (!i)
+		__raw_writel(val, isif_cfg.linear_tbl0_addr + offset);
+	else
+		__raw_writel(val, isif_cfg.linear_tbl1_addr + offset);
+}
+
+static void isif_disable_all_modules(void)
+{
+	/* disable BC */
+	regw(0, CLAMPCFG);
+	/* disable vdfc */
+	regw(0, DFCCTL);
+	/* disable CSC */
+	regw(0, CSCCTL);
+	/* disable linearization */
+	regw(0, LINCFG0);
+	/* disable other modules here as they are supported */
+}
+
+static void isif_enable(int en)
+{
+	if (!en) {
+		/* Before disable isif, disable all ISIF modules */
+		isif_disable_all_modules();
+		/*
+		 * wait for next VD. Assume lowest scan rate is 12 Hz. So
+		 * 100 msec delay is good enough
+		 */
+		msleep(100);
+	}
+	reg_modify(ISIF_SYNCEN_VDHDEN_MASK, en, SYNCEN);
+}
+
+static void isif_enable_output_to_sdram(int en)
+{
+	reg_modify(ISIF_SYNCEN_WEN_MASK, en << ISIF_SYNCEN_WEN_SHIFT, SYNCEN);
+}
+
+static void isif_config_culling(struct isif_cul *cul)
+{
+	u32 val;
+
+	/* Horizontal pattern */
+	val = (cul->hcpat_even << CULL_PAT_EVEN_LINE_SHIFT) | cul->hcpat_odd;
+	regw(val, CULH);
+
+	/* vertical pattern */
+	regw(cul->vcpat, CULV);
+
+	/* LPF */
+	reg_modify(ISIF_LPF_MASK << ISIF_LPF_SHIFT,
+		  cul->en_lpf << ISIF_LPF_SHIFT, MODESET);
+}
+
+static void isif_config_gain_offset(void)
+{
+	struct isif_gain_offsets_adj *gain_off_p =
+		&isif_cfg.bayer.config_params.gain_offset;
+	u32 val;
+
+	val = (!!gain_off_p->gain_sdram_en << GAIN_SDRAM_EN_SHIFT) |
+		(!!gain_off_p->gain_ipipe_en << GAIN_IPIPE_EN_SHIFT) |
+		(!!gain_off_p->gain_h3a_en << GAIN_H3A_EN_SHIFT) |
+		(!!gain_off_p->offset_sdram_en << OFST_SDRAM_EN_SHIFT) |
+		(!!gain_off_p->offset_ipipe_en << OFST_IPIPE_EN_SHIFT) |
+		(!!gain_off_p->offset_h3a_en << OFST_H3A_EN_SHIFT);
+
+	reg_modify(GAIN_OFFSET_EN_MASK, val, CGAMMAWD);
+
+	val = ((gain_off_p->gain.r_ye.integer & GAIN_INTEGER_MASK) <<
+		GAIN_INTEGER_SHIFT) |
+		(gain_off_p->gain.r_ye.decimal & GAIN_DECIMAL_MASK);
+	regw(val, CRGAIN);
+
+	val = ((gain_off_p->gain.gr_cy.integer & GAIN_INTEGER_MASK) <<
+		GAIN_INTEGER_SHIFT) |
+		(gain_off_p->gain.gr_cy.decimal & GAIN_DECIMAL_MASK);
+	regw(val, CGRGAIN);
+
+	val = ((gain_off_p->gain.gb_g.integer & GAIN_INTEGER_MASK) <<
+		GAIN_INTEGER_SHIFT) |
+		(gain_off_p->gain.gb_g.decimal & GAIN_DECIMAL_MASK);
+	regw(val, CGBGAIN);
+
+	val = ((gain_off_p->gain.b_mg.integer & GAIN_INTEGER_MASK) <<
+		GAIN_INTEGER_SHIFT) |
+		(gain_off_p->gain.b_mg.decimal & GAIN_DECIMAL_MASK);
+	regw(val, CBGAIN);
+
+	regw((gain_off_p->offset & OFFSET_MASK), COFSTA);
+}
+
+static void isif_restore_defaults(void)
+{
+	enum vpss_ccdc_source_sel source = VPSS_CCDCIN;
+
+	dev_dbg(isif_cfg.dev, "\nstarting isif_restore_defaults...");
+	memcpy(&isif_cfg.bayer.config_params, &isif_config_defaults,
+		sizeof(struct isif_config_params_raw));
+	/* Enable clock to ISIF, IPIPEIF and BL */
+	vpss_enable_clock(VPSS_CCDC_CLOCK, 1);
+	vpss_enable_clock(VPSS_IPIPEIF_CLOCK, 1);
+	vpss_enable_clock(VPSS_BL_CLOCK, 1);
+	/* Set default offset and gain */
+	isif_config_gain_offset();
+	vpss_select_ccdc_source(source);
+	dev_dbg(isif_cfg.dev, "\nEnd of isif_restore_defaults...");
+}
+
+static int isif_open(struct device *device)
+{
+	isif_restore_defaults();
+	return 0;
+}
+
+/* This function will configure the window size to be capture in ISIF reg */
+static void isif_setwin(struct v4l2_rect *image_win,
+			enum ccdc_frmfmt frm_fmt, int ppc)
+{
+	int horz_start, horz_nr_pixels;
+	int vert_start, vert_nr_lines;
+	int mid_img = 0;
+
+	dev_dbg(isif_cfg.dev, "\nStarting isif_setwin...");
+	/*
+	 * ppc - per pixel count. indicates how many pixels per cell
+	 * output to SDRAM. example, for ycbcr, it is one y and one c, so 2.
+	 * raw capture this is 1
+	 */
+	horz_start = image_win->left << (ppc - 1);
+	horz_nr_pixels = ((image_win->width) << (ppc - 1)) - 1;
+
+	/* Writing the horizontal info into the registers */
+	regw(horz_start & START_PX_HOR_MASK, SPH);
+	regw(horz_nr_pixels & NUM_PX_HOR_MASK, LNH);
+	vert_start = image_win->top;
+
+	if (frm_fmt == CCDC_FRMFMT_INTERLACED) {
+		vert_nr_lines = (image_win->height >> 1) - 1;
+		vert_start >>= 1;
+		/* To account for VD since line 0 doesn't have any data */
+		vert_start += 1;
+	} else {
+		/* To account for VD since line 0 doesn't have any data */
+		vert_start += 1;
+		vert_nr_lines = image_win->height - 1;
+		/* configure VDINT0 and VDINT1 */
+		mid_img = vert_start + (image_win->height / 2);
+		regw(mid_img, VDINT1);
+	}
+
+	regw(0, VDINT0);
+	regw(vert_start & START_VER_ONE_MASK, SLV0);
+	regw(vert_start & START_VER_TWO_MASK, SLV1);
+	regw(vert_nr_lines & NUM_LINES_VER, LNV);
+}
+
+static void isif_config_bclamp(struct isif_black_clamp *bc)
+{
+	u32 val;
+
+	/*
+	 * DC Offset is always added to image data irrespective of bc enable
+	 * status
+	 */
+	val = bc->dc_offset & ISIF_BC_DCOFFSET_MASK;
+	regw(val, CLDCOFST);
+
+	if (bc->en) {
+		val = (bc->bc_mode_color & ISIF_BC_MODE_COLOR_MASK) <<
+			ISIF_BC_MODE_COLOR_SHIFT;
+
+		/* Enable BC and horizontal clamp caculation paramaters */
+		val = val | 1 | ((bc->horz.mode & ISIF_HORZ_BC_MODE_MASK) <<
+			ISIF_HORZ_BC_MODE_SHIFT);
+
+		regw(val, CLAMPCFG);
+
+		if (bc->horz.mode != ISIF_HORZ_BC_DISABLE) {
+			/*
+			 * Window count for calculation
+			 * Base window selection
+			 * pixel limit
+			 * Horizontal size of window
+			 * vertical size of the window
+			 * Horizontal start position of the window
+			 * Vertical start position of the window
+			 */
+			val = (bc->horz.win_count_calc &
+				ISIF_HORZ_BC_WIN_COUNT_MASK) |
+				((!!bc->horz.base_win_sel_calc) <<
+				ISIF_HORZ_BC_WIN_SEL_SHIFT) |
+				((!!bc->horz.clamp_pix_limit) <<
+				ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
+				((bc->horz.win_h_sz_calc &
+				ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
+				ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
+				((bc->horz.win_v_sz_calc &
+				ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
+				ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
+
+			regw(val, CLHWIN0);
+
+			regw(bc->horz.win_start_h_calc &
+			     ISIF_HORZ_BC_WIN_START_H_MASK, CLHWIN1);
+
+			regw(bc->horz.win_start_v_calc &
+			     ISIF_HORZ_BC_WIN_START_V_MASK, CLHWIN2);
+		}
+
+		/* vertical clamp caculation paramaters */
+
+		/* OB H Valid */
+		val = (bc->vert.ob_h_sz_calc & ISIF_VERT_BC_OB_H_SZ_MASK);
+
+		/* Reset clamp value sel for previous line */
+		val |= ((bc->vert.reset_val_sel &
+			ISIF_VERT_BC_RST_VAL_SEL_MASK) <<
+			ISIF_VERT_BC_RST_VAL_SEL_SHIFT) |
+			(bc->vert.line_ave_coef <<
+			ISIF_VERT_BC_LINE_AVE_COEF_SHIFT);
+		regw(val, CLVWIN0);
+
+		/* Configured reset value */
+		if (bc->vert.reset_val_sel ==
+		    ISIF_VERT_BC_USE_CONFIG_CLAMP_VAL) {
+			regw(bc->vert.reset_clamp_val &
+			       ISIF_VERT_BC_RST_VAL_MASK, CLVRV);
+		}
+
+		/* Optical Black horizontal start position */
+		regw(bc->vert.ob_start_h & ISIF_VERT_BC_OB_START_HORZ_MASK,
+		     CLVWIN1);
+
+		/* Optical Black vertical start position */
+		regw(bc->vert.ob_start_v & ISIF_VERT_BC_OB_START_VERT_MASK,
+		     CLVWIN2);
+
+		regw(bc->vert.ob_v_sz_calc & ISIF_VERT_BC_OB_VERT_SZ_MASK,
+		     CLVWIN3);
+
+		/* Vertical start position for BC subtraction */
+		regw(bc->vert_start_sub & ISIF_BC_VERT_START_SUB_V_MASK, CLSV);
+	}
+}
+
+static void isif_config_linearization(struct isif_linearize *linearize)
+{
+	u32 val, i;
+
+	if (!linearize->en) {
+		regw(0, LINCFG0);
+		return;
+	}
+
+	/* shift value for correction & enable linearization (set lsb) */
+	val = (linearize->corr_shft & ISIF_LIN_CORRSFT_MASK) <<
+	       ISIF_LIN_CORRSFT_SHIFT | 1;
+	regw(val, LINCFG0);
+
+	/* Scale factor */
+	val = ((!!linearize->scale_fact.integer) <<
+	      ISIF_LIN_SCALE_FACT_INTEG_SHIFT) |
+	      (linearize->scale_fact.decimal &
+	       ISIF_LIN_SCALE_FACT_DECIMAL_MASK);
+	regw(val, LINCFG1);
+
+	for (i = 0; i < ISIF_LINEAR_TAB_SIZE; i++) {
+		val = linearize->table[i] & ISIF_LIN_ENTRY_MASK;
+		if (i % 2)
+			regw_lin_tbl(val, ((i >> 1) << 2), 1);
+		else
+			regw_lin_tbl(val, ((i >> 1) << 2), 0);
+	}
+}
+
+static int isif_config_dfc(struct isif_dfc *vdfc)
+{
+	/* initialize retries to loop for max ~ 250 usec */
+	u32 val, count, retries = loops_per_jiffy / (4000/HZ);
+	int i;
+
+	if (!vdfc->en)
+		return 0;
+
+	/* Correction mode */
+	val = ((vdfc->corr_mode & ISIF_VDFC_CORR_MOD_MASK) <<
+		ISIF_VDFC_CORR_MOD_SHIFT);
+
+	/* Correct whole line or partial */
+	if (vdfc->corr_whole_line)
+		val |= 1 << ISIF_VDFC_CORR_WHOLE_LN_SHIFT;
+
+	/* level shift value */
+	val |= (vdfc->def_level_shift & ISIF_VDFC_LEVEL_SHFT_MASK) <<
+		ISIF_VDFC_LEVEL_SHFT_SHIFT;
+
+	regw(val, DFCCTL);
+
+	/* Defect saturation level */
+	val = vdfc->def_sat_level & ISIF_VDFC_SAT_LEVEL_MASK;
+	regw(val, VDFSATLV);
+
+	regw(vdfc->table[0].pos_vert & ISIF_VDFC_POS_MASK, DFCMEM0);
+	regw(vdfc->table[0].pos_horz & ISIF_VDFC_POS_MASK, DFCMEM1);
+	if (vdfc->corr_mode == ISIF_VDFC_NORMAL ||
+	    vdfc->corr_mode == ISIF_VDFC_HORZ_INTERPOL_IF_SAT) {
+		regw(vdfc->table[0].level_at_pos, DFCMEM2);
+		regw(vdfc->table[0].level_up_pixels, DFCMEM3);
+		regw(vdfc->table[0].level_low_pixels, DFCMEM4);
+	}
+
+	/* set DFCMARST and set DFCMWR */
+	val = regr(DFCMEMCTL) | (1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT) | 1;
+	regw(val, DFCMEMCTL);
+
+	count = retries;
+	while (count && (regr(DFCMEMCTL) & 0x1))
+		count--;
+
+	if (!count) {
+		dev_dbg(isif_cfg.dev, "defect table write timeout !!!\n");
+		return -1;
+	}
+
+	for (i = 1; i < vdfc->num_vdefects; i++) {
+		regw(vdfc->table[i].pos_vert & ISIF_VDFC_POS_MASK,
+			   DFCMEM0);
+		regw(vdfc->table[i].pos_horz & ISIF_VDFC_POS_MASK,
+			   DFCMEM1);
+		if (vdfc->corr_mode == ISIF_VDFC_NORMAL ||
+		    vdfc->corr_mode == ISIF_VDFC_HORZ_INTERPOL_IF_SAT) {
+			regw(vdfc->table[i].level_at_pos, DFCMEM2);
+			regw(vdfc->table[i].level_up_pixels, DFCMEM3);
+			regw(vdfc->table[i].level_low_pixels, DFCMEM4);
+		}
+		val = regr(DFCMEMCTL);
+		/* clear DFCMARST and set DFCMWR */
+		val &= ~BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT);
+		val |= 1;
+		regw(val, DFCMEMCTL);
+
+		count = retries;
+		while (count && (regr(DFCMEMCTL) & 0x1))
+			count--;
+
+		if (!count) {
+			dev_err(isif_cfg.dev,
+				"defect table write timeout !!!\n");
+			return -1;
+		}
+	}
+	if (vdfc->num_vdefects < ISIF_VDFC_TABLE_SIZE) {
+		/* Extra cycle needed */
+		regw(0, DFCMEM0);
+		regw(0x1FFF, DFCMEM1);
+		regw(1, DFCMEMCTL);
+	}
+
+	/* enable VDFC */
+	reg_modify((1 << ISIF_VDFC_EN_SHIFT), (1 << ISIF_VDFC_EN_SHIFT),
+		   DFCCTL);
+	return 0;
+}
+
+static void isif_config_csc(struct isif_df_csc *df_csc)
+{
+	u32 val1 = 0, val2 = 0, i;
+
+	if (!df_csc->csc.en) {
+		regw(0, CSCCTL);
+		return;
+	}
+	for (i = 0; i < ISIF_CSC_NUM_COEFF; i++) {
+		if ((i % 2) == 0) {
+			/* CSCM - LSB */
+			val1 = ((df_csc->csc.coeff[i].integer &
+				ISIF_CSC_COEF_INTEG_MASK) <<
+				ISIF_CSC_COEF_INTEG_SHIFT) |
+				((df_csc->csc.coeff[i].decimal &
+				ISIF_CSC_COEF_DECIMAL_MASK));
+		} else {
+
+			/* CSCM - MSB */
+			val2 = ((df_csc->csc.coeff[i].integer &
+				ISIF_CSC_COEF_INTEG_MASK) <<
+				ISIF_CSC_COEF_INTEG_SHIFT) |
+				((df_csc->csc.coeff[i].decimal &
+				ISIF_CSC_COEF_DECIMAL_MASK));
+			val2 <<= ISIF_CSCM_MSB_SHIFT;
+			val2 |= val1;
+			regw(val2, (CSCM0 + ((i - 1) << 1)));
+		}
+	}
+
+	/* program the active area */
+	regw(df_csc->start_pix & ISIF_DF_CSC_SPH_MASK, FMTSPH);
+	/*
+	 * one extra pixel as required for CSC. Actually number of
+	 * pixel - 1 should be configured in this register. So we
+	 * need to subtract 1 before writing to FMTSPH, but we will
+	 * not do this since csc requires one extra pixel
+	 */
+	regw((df_csc->num_pixels) & ISIF_DF_CSC_SPH_MASK, FMTLNH);
+	regw(df_csc->start_line & ISIF_DF_CSC_SPH_MASK, FMTSLV);
+	/*
+	 * one extra line as required for CSC. See reason documented for
+	 * num_pixels
+	 */
+	regw((df_csc->num_lines) & ISIF_DF_CSC_SPH_MASK, FMTLNV);
+
+	/* Enable CSC */
+	regw(1, CSCCTL);
+}
+
+static int isif_config_raw(void)
+{
+	struct isif_params_raw *params = &isif_cfg.bayer;
+	struct isif_config_params_raw *module_params =
+		&isif_cfg.bayer.config_params;
+	struct vpss_pg_frame_size frame_size;
+	struct vpss_sync_pol sync;
+	u32 val;
+
+	dev_dbg(isif_cfg.dev, "\nStarting isif_config_raw..\n");
+
+	/*
+	 * Configure CCDCFG register:-
+	 * Set CCD Not to swap input since input is RAW data
+	 * Set FID detection function to Latch at V-Sync
+	 * Set WENLOG - isif valid area
+	 * Set TRGSEL
+	 * Set EXTRG
+	 * Packed to 8 or 16 bits
+	 */
+
+	val = ISIF_YCINSWP_RAW | ISIF_CCDCFG_FIDMD_LATCH_VSYNC |
+		ISIF_CCDCFG_WENLOG_AND | ISIF_CCDCFG_TRGSEL_WEN |
+		ISIF_CCDCFG_EXTRG_DISABLE | (isif_cfg.data_pack &
+		ISIF_DATA_PACK_MASK);
+
+	dev_dbg(isif_cfg.dev, "Writing 0x%x to ...CCDCFG \n", val);
+	regw(val, CCDCFG);
+
+	/*
+	 * Configure the vertical sync polarity(MODESET.VDPOL)
+	 * Configure the horizontal sync polarity (MODESET.HDPOL)
+	 * Configure frame id polarity (MODESET.FLDPOL)
+	 * Configure data polarity
+	 * Configure External WEN Selection
+	 * Configure frame format(progressive or interlace)
+	 * Configure pixel format (Input mode)
+	 * Configure the data shift
+	 */
+
+	val = ISIF_VDHDOUT_INPUT |
+		((params->vd_pol & ISIF_VD_POL_MASK) << ISIF_VD_POL_SHIFT) |
+		((params->hd_pol & ISIF_HD_POL_MASK) << ISIF_HD_POL_SHIFT) |
+		((params->fid_pol & ISIF_FID_POL_MASK) << ISIF_FID_POL_SHIFT) |
+		((ISIF_DATAPOL_NORMAL & ISIF_DATAPOL_MASK) <<
+		ISIF_DATAPOL_SHIFT) |
+		((ISIF_EXWEN_DISABLE & ISIF_EXWEN_MASK) << ISIF_EXWEN_SHIFT) |
+		((params->frm_fmt & ISIF_FRM_FMT_MASK) << ISIF_FRM_FMT_SHIFT) |
+		((params->pix_fmt & ISIF_INPUT_MASK) << ISIF_INPUT_SHIFT) |
+		((params->config_params.data_shift & ISIF_DATASFT_MASK) <<
+		ISIF_DATASFT_SHIFT);
+
+	regw(val, MODESET);
+	dev_dbg(isif_cfg.dev, "Writing 0x%x to MODESET...\n", val);
+
+	/*
+	 * Configure GAMMAWD register
+	 * CFA pattern setting
+	 */
+	val = (params->cfa_pat & ISIF_GAMMAWD_CFA_MASK) <<
+		ISIF_GAMMAWD_CFA_SHIFT;
+
+	/* Gamma msb */
+	if (module_params->compress.alg == ISIF_ALAW)
+		val |= ISIF_ALAW_ENABLE;
+
+	val |= ((params->data_msb & ISIF_ALAW_GAMA_WD_MASK) <<
+			ISIF_ALAW_GAMA_WD_SHIFT);
+
+	regw(val, CGAMMAWD);
+
+	/* Configure DPCM compression settings */
+	if (module_params->compress.alg == ISIF_DPCM) {
+		val =  BIT(ISIF_DPCM_EN_SHIFT) |
+		       ((module_params->compress.pred &
+			ISIF_DPCM_PREDICTOR_MASK) << ISIF_DPCM_PREDICTOR_SHIFT);
+	}
+
+	regw(val, MISC);
+
+	/* Configure Gain & Offset */
+	isif_config_gain_offset();
+
+	/* Configure Color pattern */
+	val = (params->config_params.col_pat_field0.olop) |
+	(params->config_params.col_pat_field0.olep << 2) |
+	(params->config_params.col_pat_field0.elop << 4) |
+	(params->config_params.col_pat_field0.elep << 6) |
+	(params->config_params.col_pat_field1.olop << 8) |
+	(params->config_params.col_pat_field1.olep << 10) |
+	(params->config_params.col_pat_field1.elop << 12) |
+	(params->config_params.col_pat_field1.elep << 14);
+	regw(val, CCOLP);
+	dev_dbg(isif_cfg.dev, "Writing %x to CCOLP ...\n", val);
+
+	/* Configure HSIZE register  */
+	val = (params->horz_flip_en & ISIF_HSIZE_FLIP_MASK) <<
+		ISIF_HSIZE_FLIP_SHIFT;
+
+	/* calculate line offset in 32 bytes based on pack value */
+	if (isif_cfg.data_pack == ISIF_PACK_8BIT)
+		val |= (((params->win.width + 31) >> 5) & ISIF_LINEOFST_MASK);
+	else if (isif_cfg.data_pack == ISIF_PACK_12BIT)
+		val |= ((((params->win.width +
+			(params->win.width >> 2)) + 31) >> 5) &
+			ISIF_LINEOFST_MASK);
+	else
+		val |= ((((params->win.width * 2) + 31) >> 5) &
+			ISIF_LINEOFST_MASK);
+	regw(val, HSIZE);
+
+	/* Configure SDOFST register  */
+	if (params->frm_fmt == CCDC_FRMFMT_INTERLACED) {
+		if (params->image_invert_en) {
+			/* For interlace inverse mode */
+			regw(0x4B6D, SDOFST);
+			dev_dbg(isif_cfg.dev, "Writing 0x4B6D to SDOFST...\n");
+		} else {
+			/* For interlace non inverse mode */
+			regw(0x0B6D, SDOFST);
+			dev_dbg(isif_cfg.dev, "Writing 0x0B6D to SDOFST...\n");
+		}
+	} else if (params->frm_fmt == CCDC_FRMFMT_PROGRESSIVE) {
+		if (params->image_invert_en) {
+			/* For progessive inverse mode */
+			regw(0x4000, SDOFST);
+			dev_dbg(isif_cfg.dev, "Writing 0x4000 to SDOFST...\n");
+		} else {
+			/* For progessive non inverse mode */
+			regw(0x0000, SDOFST);
+			dev_dbg(isif_cfg.dev, "Writing 0x0000 to SDOFST...\n");
+		}
+	}
+
+	/* Configure video window */
+	isif_setwin(&params->win, params->frm_fmt, 1);
+
+	/* Configure Black Clamp */
+	isif_config_bclamp(&module_params->bclamp);
+
+	/* Configure Vertical Defection Pixel Correction */
+	if (isif_config_dfc(&module_params->dfc) < 0)
+		return -EFAULT;
+
+	if (!module_params->df_csc.df_or_csc)
+		/* Configure Color Space Conversion */
+		isif_config_csc(&module_params->df_csc);
+
+	isif_config_linearization(&module_params->linearize);
+
+	/* Configure Culling */
+	isif_config_culling(&module_params->culling);
+
+	/* Configure Horizontal and vertical offsets(DFC,LSC,Gain) */
+	val = module_params->horz_offset & ISIF_DATA_H_OFFSET_MASK;
+	regw(val, DATAHOFST);
+
+	val = module_params->vert_offset & ISIF_DATA_V_OFFSET_MASK;
+	regw(val, DATAVOFST);
+
+	/* Setup test pattern if enabled */
+	if (params->config_params.test_pat_gen) {
+		/* Use the HD/VD pol settings from user */
+		sync.ccdpg_hdpol = params->hd_pol & ISIF_HD_POL_MASK;
+		sync.ccdpg_vdpol = params->vd_pol & ISIF_VD_POL_MASK;
+
+		dm365_vpss_set_sync_pol(sync);
+
+		frame_size.hlpfr = isif_cfg.bayer.win.width;
+		frame_size.pplen = isif_cfg.bayer.win.height;
+		dm365_vpss_set_pg_frame_size(frame_size);
+		vpss_select_ccdc_source(VPSS_PGLPBK);
+	}
+
+	dev_dbg(isif_cfg.dev, "\nEnd of isif_config_ycbcr...\n");
+	return 0;
+}
+
+static int isif_validate_df_csc_params(struct isif_df_csc *df_csc)
+{
+	struct isif_color_space_conv *csc;
+	int i, csc_df_en = 0;
+	int err = -EINVAL;
+
+	if (!df_csc->df_or_csc) {
+		/* csc configuration */
+		csc = &df_csc->csc;
+		if (csc->en) {
+			csc_df_en = 1;
+			for (i = 0; i < ISIF_CSC_NUM_COEFF; i++) {
+				if (csc->coeff[i].integer >
+					ISIF_CSC_COEF_INTEG_MASK ||
+				    csc->coeff[i].decimal >
+					ISIF_CSC_COEF_DECIMAL_MASK) {
+					dev_dbg(isif_cfg.dev,
+					       "invalid csc coefficients \n");
+					return err;
+				}
+			}
+		}
+	}
+
+	if (df_csc->start_pix > ISIF_DF_CSC_SPH_MASK) {
+		dev_dbg(isif_cfg.dev, "invalid df_csc start pix value \n");
+		return err;
+	}
+	if (df_csc->num_pixels > ISIF_DF_NUMPIX) {
+		dev_dbg(isif_cfg.dev, "invalid df_csc num pixels value \n");
+		return err;
+	}
+	if (df_csc->start_line > ISIF_DF_CSC_LNH_MASK) {
+		dev_dbg(isif_cfg.dev, "invalid df_csc start_line value \n");
+		return err;
+	}
+	if (df_csc->num_lines > ISIF_DF_NUMLINES) {
+		dev_dbg(isif_cfg.dev, "invalid df_csc num_lines value \n");
+		return err;
+	}
+	return 0;
+}
+
+static int isif_validate_dfc_params(struct isif_dfc *dfc)
+{
+	int err = -EINVAL;
+	int i;
+
+	if (dfc->en) {
+		if (dfc->corr_whole_line > 1) {
+			dev_dbg(isif_cfg.dev,
+				"invalid corr_whole_line value\n");
+			return err;
+		}
+
+		if (dfc->def_level_shift > 4) {
+			dev_dbg(isif_cfg.dev,
+				"invalid def_level_shift value\n");
+			return err;
+		}
+
+		if (dfc->def_sat_level > 4095) {
+			dev_dbg(isif_cfg.dev, "invalid def_sat_level value \n");
+			return err;
+		}
+		if ((!dfc->num_vdefects) || (dfc->num_vdefects > 8)) {
+			dev_dbg(isif_cfg.dev, "invalid num_vdefects value \n");
+			return err;
+		}
+		for (i = 0; i < ISIF_VDFC_TABLE_SIZE; i++) {
+			if (dfc->table[i].pos_vert > 0x1fff) {
+				dev_dbg(isif_cfg.dev,
+					"invalid pos_vert value \n");
+				return err;
+			}
+			if (dfc->table[i].pos_horz > 0x1fff) {
+				dev_dbg(isif_cfg.dev,
+					"invalid pos_horz value \n");
+				return err;
+			}
+		}
+	}
+	return 0;
+}
+
+static int isif_validate_bclamp_params(struct isif_black_clamp *bclamp)
+{
+	int err = -EINVAL;
+
+	if (bclamp->dc_offset > 0x1fff) {
+		dev_dbg(isif_cfg.dev, "invalid bclamp dc_offset value \n");
+		return err;
+	}
+
+	if (bclamp->en) {
+		if (bclamp->horz.clamp_pix_limit > 1) {
+			dev_dbg(isif_cfg.dev,
+			       "invalid bclamp horz clamp_pix_limit value \n");
+			return err;
+		}
+
+		if (bclamp->horz.win_count_calc < 1 ||
+		    bclamp->horz.win_count_calc > 32) {
+			dev_dbg(isif_cfg.dev,
+			       "invalid bclamp horz win_count_calc value \n");
+			return err;
+		}
+
+		if (bclamp->horz.win_start_h_calc > 0x1fff) {
+			dev_dbg(isif_cfg.dev,
+			       "invalid bclamp win_start_v_calc value \n");
+			return err;
+		}
+
+		if (bclamp->horz.win_start_v_calc > 0x1fff) {
+			dev_dbg(isif_cfg.dev,
+			       "invalid bclamp win_start_v_calc value \n");
+			return err;
+		}
+
+		if (bclamp->vert.reset_clamp_val > 0xfff) {
+			dev_dbg(isif_cfg.dev,
+			       "invalid bclamp reset_clamp_val value \n");
+			return err;
+		}
+
+		if (bclamp->vert.ob_v_sz_calc > 0x1fff) {
+			dev_dbg(isif_cfg.dev,
+				"invalid bclamp ob_v_sz_calc value \n");
+			return err;
+		}
+
+		if (bclamp->vert.ob_start_h > 0x1fff) {
+			dev_dbg(isif_cfg.dev,
+				"invalid bclamp ob_start_h value \n");
+			return err;
+		}
+
+		if (bclamp->vert.ob_start_v > 0x1fff) {
+			dev_dbg(isif_cfg.dev,
+				"invalid bclamp ob_start_h value \n");
+			return err;
+		}
+	}
+	return 0;
+}
+
+static int isif_validate_gain_ofst_params(struct isif_gain_offsets_adj
+					  *gain_offset)
+{
+	int err = -EINVAL;
+
+	if (gain_offset->gain_sdram_en ||
+	    gain_offset->gain_ipipe_en ||
+	    gain_offset->gain_h3a_en) {
+		if ((gain_offset->gain.r_ye.integer > 7) ||
+		    (gain_offset->gain.r_ye.decimal > 0x1ff)) {
+			dev_dbg(isif_cfg.dev, "invalid  gain r_ye\n");
+			return err;
+		}
+		if ((gain_offset->gain.gr_cy.integer > 7) ||
+		    (gain_offset->gain.gr_cy.decimal > 0x1ff)) {
+			dev_dbg(isif_cfg.dev, "invalid  gain gr_cy\n");
+			return err;
+		}
+		if ((gain_offset->gain.gb_g.integer > 7) ||
+		    (gain_offset->gain.gb_g.decimal > 0x1ff)) {
+			dev_dbg(isif_cfg.dev, "invalid  gain gb_g\n");
+			return err;
+		}
+		if ((gain_offset->gain.b_mg.integer > 7) ||
+		    (gain_offset->gain.b_mg.decimal > 0x1ff)) {
+			dev_dbg(isif_cfg.dev, "invalid  gain b_mg\n");
+			return err;
+		}
+	}
+	if (gain_offset->offset_sdram_en ||
+	    gain_offset->offset_ipipe_en ||
+	    gain_offset->offset_h3a_en) {
+		if (gain_offset->offset > 0xfff) {
+			dev_dbg(isif_cfg.dev, "invalid  gain b_mg\n");
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int
+validate_isif_config_params_raw(struct isif_config_params_raw *params)
+{
+	int err;
+
+	err = isif_validate_df_csc_params(&params->df_csc);
+	if (err)
+		goto exit;
+	err = isif_validate_dfc_params(&params->dfc);
+	if (err)
+		goto exit;
+	err = isif_validate_bclamp_params(&params->bclamp);
+	if (err)
+		goto exit;
+	err = isif_validate_gain_ofst_params(&params->gain_offset);
+exit:
+	return err;
+}
+
+static int isif_set_buftype(enum ccdc_buftype buf_type)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		isif_cfg.bayer.buf_type = buf_type;
+	else
+		isif_cfg.ycbcr.buf_type = buf_type;
+
+	return 0;
+
+}
+static enum ccdc_buftype isif_get_buftype(void)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		return isif_cfg.bayer.buf_type;
+
+	return isif_cfg.ycbcr.buf_type;
+}
+
+static int isif_enum_pix(u32 *pix, int i)
+{
+	int ret = -EINVAL;
+
+	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
+		if (i < ARRAY_SIZE(isif_raw_bayer_pix_formats)) {
+			*pix = isif_raw_bayer_pix_formats[i];
+			ret = 0;
+		}
+	} else {
+		if (i < ARRAY_SIZE(isif_raw_yuv_pix_formats)) {
+			*pix = isif_raw_yuv_pix_formats[i];
+			ret = 0;
+		}
+	}
+
+	return ret;
+}
+
+static int isif_set_pixel_format(unsigned int pixfmt)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
+		if (pixfmt == V4L2_PIX_FMT_SBGGR8) {
+			if ((isif_cfg.bayer.config_params.compress.alg !=
+			     ISIF_ALAW) &&
+			    (isif_cfg.bayer.config_params.compress.alg !=
+			     ISIF_DPCM)) {
+				dev_dbg(isif_cfg.dev,
+					"Either configure A-Law or DPCM\n");
+				return -EINVAL;
+			}
+			isif_cfg.data_pack = ISIF_PACK_8BIT;
+		} else if (pixfmt == V4L2_PIX_FMT_SBGGR16) {
+			isif_cfg.bayer.config_params.compress.alg =
+					ISIF_NO_COMPRESSION;
+			isif_cfg.data_pack = ISIF_PACK_16BIT;
+		} else
+			return -EINVAL;
+		isif_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
+	} else {
+		if (pixfmt == V4L2_PIX_FMT_YUYV)
+			isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_YCBYCR;
+		else if (pixfmt == V4L2_PIX_FMT_UYVY)
+			isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
+		else
+			return -EINVAL;
+		isif_cfg.data_pack = ISIF_PACK_8BIT;
+	}
+	return 0;
+}
+
+static u32 isif_get_pixel_format(void)
+{
+	u32 pixfmt;
+
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		if (isif_cfg.bayer.config_params.compress.alg
+			== ISIF_ALAW
+			|| isif_cfg.bayer.config_params.compress.alg
+			== ISIF_DPCM)
+				pixfmt = V4L2_PIX_FMT_SBGGR8;
+		else
+			pixfmt = V4L2_PIX_FMT_SBGGR16;
+	else {
+		if (isif_cfg.ycbcr.pix_order == CCDC_PIXORDER_YCBYCR)
+			pixfmt = V4L2_PIX_FMT_YUYV;
+		else
+			pixfmt = V4L2_PIX_FMT_UYVY;
+	}
+	return pixfmt;
+}
+
+static int isif_set_image_window(struct v4l2_rect *win)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
+		isif_cfg.bayer.win.top = win->top;
+		isif_cfg.bayer.win.left = win->left;
+		isif_cfg.bayer.win.width = win->width;
+		isif_cfg.bayer.win.height = win->height;
+	} else {
+		isif_cfg.ycbcr.win.top = win->top;
+		isif_cfg.ycbcr.win.left = win->left;
+		isif_cfg.ycbcr.win.width = win->width;
+		isif_cfg.ycbcr.win.height = win->height;
+	}
+	return 0;
+}
+
+static void isif_get_image_window(struct v4l2_rect *win)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		*win = isif_cfg.bayer.win;
+	else
+		*win = isif_cfg.ycbcr.win;
+}
+
+static unsigned int isif_get_line_length(void)
+{
+	unsigned int len;
+
+	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
+		if (isif_cfg.data_pack == ISIF_PACK_8BIT)
+			len = ((isif_cfg.bayer.win.width));
+		else if (isif_cfg.data_pack == ISIF_PACK_12BIT)
+			len = (((isif_cfg.bayer.win.width * 2) +
+				 (isif_cfg.bayer.win.width >> 2)));
+		else
+			len = (((isif_cfg.bayer.win.width * 2)));
+	} else
+		len = (((isif_cfg.ycbcr.win.width * 2)));
+
+	return ALIGN(len, 32);
+}
+
+static int isif_set_frame_format(enum ccdc_frmfmt frm_fmt)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		isif_cfg.bayer.frm_fmt = frm_fmt;
+	else
+		isif_cfg.ycbcr.frm_fmt = frm_fmt;
+
+	return 0;
+}
+static enum ccdc_frmfmt isif_get_frame_format(void)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		return isif_cfg.bayer.frm_fmt;
+	else
+		return isif_cfg.ycbcr.frm_fmt;
+}
+
+static int isif_getfid(void)
+{
+	return (regr(MODESET) >> 15) & 0x1;
+}
+
+/* misc operations */
+static void isif_setfbaddr(unsigned long addr)
+{
+	regw((addr >> 21) & 0x07ff, CADU);
+	regw((addr >> 5) & 0x0ffff, CADL);
+}
+
+static int isif_set_hw_if_params(struct vpfe_hw_if_param *params)
+{
+	isif_cfg.if_type = params->if_type;
+
+	switch (params->if_type) {
+	case VPFE_BT656:
+	case VPFE_BT656_10BIT:
+	case VPFE_YCBCR_SYNC_8:
+		isif_cfg.ycbcr.pix_fmt = CCDC_PIXFMT_YCBCR_8BIT;
+		isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
+		break;
+	case VPFE_BT1120:
+	case VPFE_YCBCR_SYNC_16:
+		isif_cfg.ycbcr.pix_fmt = CCDC_PIXFMT_YCBCR_16BIT;
+		isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
+		break;
+	case VPFE_RAW_BAYER:
+		isif_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
+		break;
+	default:
+		dev_dbg(isif_cfg.dev, "Invalid interface type\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Parameter operations */
+static int isif_get_params(void __user *params)
+{
+	/* only raw module parameters can be set through the IOCTL */
+	if (isif_cfg.if_type != VPFE_RAW_BAYER)
+		return -EINVAL;
+
+	if (copy_to_user(params,
+			&isif_cfg.bayer.config_params,
+			sizeof(isif_cfg.bayer.config_params))) {
+		dev_dbg(isif_cfg.dev,
+			"isif_get_params: error in copying isif params\n");
+		return -EFAULT;
+	}
+	return 0;
+}
+
+/* Parameter operations */
+static int isif_set_params(void __user *params)
+{
+	struct isif_config_params_raw *isif_raw_params;
+	int ret = -EINVAL;
+
+	/* only raw module parameters can be set through the IOCTL */
+	if (isif_cfg.if_type != VPFE_RAW_BAYER)
+		return ret;
+
+	isif_raw_params = kzalloc(sizeof(*isif_raw_params), GFP_KERNEL);
+
+	if (NULL == isif_raw_params)
+		return -ENOMEM;
+
+	ret = copy_from_user(isif_raw_params,
+			     params, sizeof(*isif_raw_params));
+	if (ret) {
+		dev_dbg(isif_cfg.dev, "isif_set_params: error in copying isif"
+			"params, %d\n", ret);
+		ret = -EFAULT;
+		goto free_out;
+	}
+
+	if (!validate_isif_config_params_raw(isif_raw_params)) {
+		memcpy(&isif_cfg.bayer.config_params,
+			isif_raw_params,
+			sizeof(*isif_raw_params));
+		ret = 0;
+	} else
+		ret = -EINVAL;
+free_out:
+	kfree(isif_raw_params);
+	return ret;
+}
+
+/* This function will configure ISIF for YCbCr parameters. */
+static int isif_config_ycbcr(void)
+{
+	struct isif_ycbcr_config *params = &isif_cfg.ycbcr;
+	struct vpss_pg_frame_size frame_size;
+	u32 modeset = 0, ccdcfg = 0;
+	struct vpss_sync_pol sync;
+
+	dev_dbg(isif_cfg.dev, "\nStarting isif_config_ycbcr...");
+
+	/* configure pixel format or input mode */
+	modeset = modeset | ((params->pix_fmt & ISIF_INPUT_MASK)
+		<< ISIF_INPUT_SHIFT) |
+	((params->frm_fmt & ISIF_FRM_FMT_MASK) << ISIF_FRM_FMT_SHIFT) |
+	(((params->fid_pol & ISIF_FID_POL_MASK) << ISIF_FID_POL_SHIFT))	|
+	(((params->hd_pol & ISIF_HD_POL_MASK) << ISIF_HD_POL_SHIFT)) |
+	(((params->vd_pol & ISIF_VD_POL_MASK) << ISIF_VD_POL_SHIFT));
+
+	/* pack the data to 8-bit ISIFCFG */
+	switch (isif_cfg.if_type) {
+	case VPFE_BT656:
+		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
+			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
+			return -1;
+		}
+		modeset |=
+			((VPFE_PINPOL_NEGATIVE & ISIF_VD_POL_MASK)
+			<< ISIF_VD_POL_SHIFT);
+		regw(3, REC656IF);
+		ccdcfg = ccdcfg | ISIF_DATA_PACK8 | ISIF_YCINSWP_YCBCR;
+		break;
+	case VPFE_BT656_10BIT:
+		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
+			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
+			return -1;
+		}
+		/* setup BT.656, embedded sync  */
+		regw(3, REC656IF);
+		/* enable 10 bit mode in ccdcfg */
+		ccdcfg = ccdcfg | ISIF_DATA_PACK8 | ISIF_YCINSWP_YCBCR |
+			ISIF_BW656_ENABLE;
+		break;
+	case VPFE_BT1120:
+		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_16BIT) {
+			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
+			return -EINVAL;
+		}
+		regw(3, REC656IF);
+		break;
+
+	case VPFE_YCBCR_SYNC_8:
+		ccdcfg |= ISIF_DATA_PACK8;
+		ccdcfg |= ISIF_YCINSWP_YCBCR;
+		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
+			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
+			return -EINVAL;
+		}
+		break;
+	case VPFE_YCBCR_SYNC_16:
+		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_16BIT) {
+			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
+			return -EINVAL;
+		}
+		break;
+	default:
+		/* should never come here */
+		dev_dbg(isif_cfg.dev, "Invalid interface type\n");
+		return -EINVAL;
+	}
+
+	regw(modeset, MODESET);
+
+	/* Set up pix order */
+	ccdcfg |= (params->pix_order & ISIF_PIX_ORDER_MASK) <<
+		ISIF_PIX_ORDER_SHIFT;
+
+	regw(ccdcfg, CCDCFG);
+
+	/* configure video window */
+	if ((isif_cfg.if_type == VPFE_BT1120) ||
+	    (isif_cfg.if_type == VPFE_YCBCR_SYNC_16))
+		isif_setwin(&params->win, params->frm_fmt, 1);
+	else
+		isif_setwin(&params->win, params->frm_fmt, 2);
+
+	/*
+	 * configure the horizontal line offset
+	 * this is done by rounding up width to a multiple of 16 pixels
+	 * and multiply by two to account for y:cb:cr 4:2:2 data
+	 */
+	regw(((((params->win.width * 2) + 31) & 0xffffffe0) >> 5), HSIZE);
+
+	/* configure the memory line offset */
+	if ((params->frm_fmt == CCDC_FRMFMT_INTERLACED) &&
+	    (params->buf_type == CCDC_BUFTYPE_FLD_INTERLEAVED))
+		/* two fields are interleaved in memory */
+		regw(0x00000249, SDOFST);
+
+	/* Setup test pattern if enabled */
+	if (isif_cfg.bayer.config_params.test_pat_gen) {
+		sync.ccdpg_hdpol = (params->hd_pol & ISIF_HD_POL_MASK);
+		sync.ccdpg_vdpol = (params->vd_pol & ISIF_VD_POL_MASK);
+		dm365_vpss_set_sync_pol(sync);
+		dm365_vpss_set_pg_frame_size(frame_size);
+	}
+
+	return 0;
+}
+
+static int isif_configure(void)
+{
+	if (isif_cfg.if_type == VPFE_RAW_BAYER)
+		return isif_config_raw();
+	else
+		isif_config_ycbcr();
+	return 0;
+}
+
+static int isif_close(struct device *device)
+{
+	/* copy defaults to module params */
+	memcpy(&isif_cfg.bayer.config_params,
+	       &isif_config_defaults,
+	       sizeof(struct isif_config_params_raw));
+	return 0;
+}
+
+static struct ccdc_hw_device isif_hw_dev = {
+	.name = "ISIF",
+	.owner = THIS_MODULE,
+	.hw_ops = {
+		.open = isif_open,
+		.close = isif_close,
+		.enable = isif_enable,
+		.enable_out_to_sdram = isif_enable_output_to_sdram,
+		.set_hw_if_params = isif_set_hw_if_params,
+		.set_params = isif_set_params,
+		.get_params = isif_get_params,
+		.configure = isif_configure,
+		.set_buftype = isif_set_buftype,
+		.get_buftype = isif_get_buftype,
+		.enum_pix = isif_enum_pix,
+		.set_pixel_format = isif_set_pixel_format,
+		.get_pixel_format = isif_get_pixel_format,
+		.set_frame_format = isif_set_frame_format,
+		.get_frame_format = isif_get_frame_format,
+		.set_image_window = isif_set_image_window,
+		.get_image_window = isif_get_image_window,
+		.get_line_length = isif_get_line_length,
+		.setfbaddr = isif_setfbaddr,
+		.getfid = isif_getfid,
+	},
+};
+
+static int __init isif_probe(struct platform_device *pdev)
+{
+	void (*setup_pinmux)(void);
+	struct resource	*res;
+	void *__iomem addr;
+	int status = 0, i;
+
+	/*
+	 * first try to register with vpfe. If not correct platform, then we
+	 * don't have to iomap
+	 */
+	status = vpfe_register_ccdc_device(&isif_hw_dev);
+	if (status < 0)
+		return status;
+
+	/* Get and enable Master clock */
+	isif_cfg.mclk = clk_get(&pdev->dev, "master");
+	if (NULL == isif_cfg.mclk) {
+		status = -ENODEV;
+		goto fail_mclk;
+	}
+	if (clk_enable(isif_cfg.mclk)) {
+		status = -ENODEV;
+		goto fail_mclk;
+	}
+
+	/* Platform data holds setup_pinmux function ptr */
+	if (NULL == pdev->dev.platform_data) {
+		status = -ENODEV;
+		goto fail_mclk;
+	}
+	setup_pinmux = pdev->dev.platform_data;
+	/*
+	 * setup Mux configuration for ccdc which may be different for
+	 * different SoCs using this CCDC
+	 */
+	setup_pinmux();
+
+	i = 0;
+	/* Get the ISIF base address, linearization table0 and table1 addr. */
+	while (i < 3) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (!res) {
+			status = -ENODEV;
+			goto fail_nobase_res;
+		}
+		res = request_mem_region(res->start, resource_size(res),
+					 res->name);
+		if (!res) {
+			status = -EBUSY;
+			goto fail_nobase_res;
+		}
+		addr = ioremap_nocache(res->start, resource_size(res));
+		if (!addr) {
+			status = -ENOMEM;
+			goto fail_base_iomap;
+		}
+		switch (i) {
+		case 0:
+			/* ISIF base address */
+			isif_cfg.base_addr = addr;
+			break;
+		case 1:
+			/* ISIF linear tbl0 address */
+			isif_cfg.linear_tbl0_addr = addr;
+			break;
+		default:
+			/* ISIF linear tbl0 address */
+			isif_cfg.linear_tbl1_addr = addr;
+			break;
+		}
+		i++;
+	}
+	isif_cfg.dev = &pdev->dev;
+
+	printk(KERN_NOTICE "%s is registered with vpfe.\n",
+		isif_hw_dev.name);
+	return 0;
+fail_base_iomap:
+	release_mem_region(res->start, resource_size(res));
+	i--;
+fail_nobase_res:
+	if (isif_cfg.base_addr)
+		iounmap(isif_cfg.base_addr);
+	if (isif_cfg.linear_tbl0_addr)
+		iounmap(isif_cfg.linear_tbl0_addr);
+
+	while (i >= 0) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		release_mem_region(res->start, resource_size(res));
+		i--;
+	}
+fail_mclk:
+	clk_put(isif_cfg.mclk);
+	vpfe_unregister_ccdc_device(&isif_hw_dev);
+	return status;
+}
+
+static int isif_remove(struct platform_device *pdev)
+{
+	struct resource	*res;
+	int i = 0;
+
+	iounmap(isif_cfg.base_addr);
+	iounmap(isif_cfg.linear_tbl0_addr);
+	iounmap(isif_cfg.linear_tbl1_addr);
+	while (i < 3) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		if (res)
+			release_mem_region(res->start, resource_size(res));
+		i++;
+	}
+	vpfe_unregister_ccdc_device(&isif_hw_dev);
+	return 0;
+}
+
+static struct platform_driver isif_driver = {
+	.driver = {
+		.name	= "isif",
+		.owner = THIS_MODULE,
+	},
+	.remove = __devexit_p(isif_remove),
+	.probe = isif_probe,
+};
+
+static int __init isif_init(void)
+{
+	return platform_driver_register(&isif_driver);
+}
+
+static void isif_exit(void)
+{
+	platform_driver_unregister(&isif_driver);
+}
+
+module_init(isif_init);
+module_exit(isif_exit);
+
+MODULE_LICENSE("GPL");
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH - v1 1/6] V4L - vpfe-capture : DM365 vpss enhancements
  2009-12-10 17:00   ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source m-karicheri2
@ 2009-12-10 17:00     ` m-karicheri2
  2009-12-10 17:00       ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver m-karicheri2
  2009-12-15  7:57     ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source Hans Verkuil
  1 sibling, 1 reply; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

Enhancements to support DM365 ISP5 and VPSS module configuration.
Also cleaned up the driver by removing redundant variables.

Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
Applies to linux-next v4l-dvb tree
 drivers/media/video/davinci/vpss.c |  289 +++++++++++++++++++++++++++++-------
 include/media/davinci/vpss.h       |   41 +++++-
 2 files changed, 275 insertions(+), 55 deletions(-)

diff --git a/drivers/media/video/davinci/vpss.c b/drivers/media/video/davinci/vpss.c
index 6d709ca..03f625d 100644
--- a/drivers/media/video/davinci/vpss.c
+++ b/drivers/media/video/davinci/vpss.c
@@ -15,7 +15,7 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  *
- * common vpss driver for all video drivers.
+ * common vpss system module platform driver for all video drivers.
  */
 #include <linux/kernel.h>
 #include <linux/sched.h>
@@ -35,12 +35,52 @@ MODULE_AUTHOR("Texas Instruments");
 /* DM644x defines */
 #define DM644X_SBL_PCR_VPSS		(4)
 
+#define DM355_VPSSBL_INTSEL		0x10
+#define DM355_VPSSBL_EVTSEL		0x14
 /* vpss BL register offsets */
 #define DM355_VPSSBL_CCDCMUX		0x1c
 /* vpss CLK register offsets */
 #define DM355_VPSSCLK_CLKCTRL		0x04
 /* masks and shifts */
 #define VPSS_HSSISEL_SHIFT		4
+/*
+ * VDINT0 - vpss_int0, VDINT1 - vpss_int1, H3A - vpss_int4,
+ * IPIPE_INT1_SDR - vpss_int5
+ */
+#define DM355_VPSSBL_INTSEL_DEFAULT	0xff83ff10
+/* VENCINT - vpss_int8 */
+#define DM355_VPSSBL_EVTSEL_DEFAULT	0x4
+
+#define DM365_ISP5_PCCR 		0x04
+#define DM365_ISP5_INTSEL1		0x10
+#define DM365_ISP5_INTSEL2		0x14
+#define DM365_ISP5_INTSEL3		0x18
+#define DM365_ISP5_CCDCMUX 		0x20
+#define DM365_ISP5_PG_FRAME_SIZE 	0x28
+#define DM365_VPBE_CLK_CTRL 		0x00
+/*
+ * vpss interrupts. VDINT0 - vpss_int0, VDINT1 - vpss_int1,
+ * AF - vpss_int3
+ */
+#define DM365_ISP5_INTSEL1_DEFAULT	0x0b1f0100
+/* AEW - vpss_int6, RSZ_INT_DMA - vpss_int5 */
+#define DM365_ISP5_INTSEL2_DEFAULT	0x1f0a0f1f
+/* VENC - vpss_int8 */
+#define DM365_ISP5_INTSEL3_DEFAULT	0x00000015
+
+/* masks and shifts for DM365*/
+#define DM365_CCDC_PG_VD_POL_SHIFT 	0
+#define DM365_CCDC_PG_HD_POL_SHIFT 	1
+
+#define CCD_SRC_SEL_MASK		(BIT_MASK(5) | BIT_MASK(4))
+#define CCD_SRC_SEL_SHIFT		4
+
+/* Different SoC platforms supported by this driver */
+enum vpss_platform_type {
+	DM644X,
+	DM355,
+	DM365,
+};
 
 /*
  * vpss operations. Depends on platform. Not all functions are available
@@ -59,13 +99,9 @@ struct vpss_hw_ops {
 
 /* vpss configuration */
 struct vpss_oper_config {
-	__iomem void *vpss_bl_regs_base;
-	__iomem void *vpss_regs_base;
-	struct resource		*r1;
-	resource_size_t		len1;
-	struct resource		*r2;
-	resource_size_t		len2;
-	char vpss_name[32];
+	__iomem void *vpss_regs_base0;
+	__iomem void *vpss_regs_base1;
+	enum vpss_platform_type platform;
 	spinlock_t vpss_lock;
 	struct vpss_hw_ops hw_ops;
 };
@@ -75,22 +111,46 @@ static struct vpss_oper_config oper_cfg;
 /* register access routines */
 static inline u32 bl_regr(u32 offset)
 {
-	return __raw_readl(oper_cfg.vpss_bl_regs_base + offset);
+	return __raw_readl(oper_cfg.vpss_regs_base0 + offset);
 }
 
 static inline void bl_regw(u32 val, u32 offset)
 {
-	__raw_writel(val, oper_cfg.vpss_bl_regs_base + offset);
+	__raw_writel(val, oper_cfg.vpss_regs_base0 + offset);
 }
 
 static inline u32 vpss_regr(u32 offset)
 {
-	return __raw_readl(oper_cfg.vpss_regs_base + offset);
+	return __raw_readl(oper_cfg.vpss_regs_base1 + offset);
 }
 
 static inline void vpss_regw(u32 val, u32 offset)
 {
-	__raw_writel(val, oper_cfg.vpss_regs_base + offset);
+	__raw_writel(val, oper_cfg.vpss_regs_base1 + offset);
+}
+
+/* For DM365 only */
+static inline u32 isp5_read(u32 offset)
+{
+	return __raw_readl(oper_cfg.vpss_regs_base0 + offset);
+}
+
+/* For DM365 only */
+static inline void isp5_write(u32 val, u32 offset)
+{
+	__raw_writel(val, oper_cfg.vpss_regs_base0 + offset);
+}
+
+static void dm365_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
+{
+	u32 temp = isp5_read(DM365_ISP5_CCDCMUX) & ~CCD_SRC_SEL_MASK;
+
+	/* if we are using pattern generator, enable it */
+	if (src_sel == VPSS_PGLPBK || src_sel == VPSS_CCDCPG)
+		temp |= 0x08;
+
+	temp |= (src_sel << CCD_SRC_SEL_SHIFT);
+	isp5_write(temp, DM365_ISP5_CCDCMUX);
 }
 
 static void dm355_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
@@ -101,9 +161,9 @@ static void dm355_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
 int vpss_select_ccdc_source(enum vpss_ccdc_source_sel src_sel)
 {
 	if (!oper_cfg.hw_ops.select_ccdc_source)
-		return -1;
+		return -EINVAL;
 
-	dm355_select_ccdc_source(src_sel);
+	oper_cfg.hw_ops.select_ccdc_source(src_sel);
 	return 0;
 }
 EXPORT_SYMBOL(vpss_select_ccdc_source);
@@ -114,7 +174,7 @@ static int dm644x_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel)
 
 	if (wbl_sel < VPSS_PCR_AEW_WBL_0 ||
 	    wbl_sel > VPSS_PCR_CCDC_WBL_O)
-		return -1;
+		return -EINVAL;
 
 	/* writing a 0 clear the overflow */
 	mask = ~(mask << wbl_sel);
@@ -126,7 +186,7 @@ static int dm644x_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel)
 int vpss_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel)
 {
 	if (!oper_cfg.hw_ops.clear_wbl_overflow)
-		return -1;
+		return -EINVAL;
 
 	return oper_cfg.hw_ops.clear_wbl_overflow(wbl_sel);
 }
@@ -166,7 +226,7 @@ static int dm355_enable_clock(enum vpss_clock_sel clock_sel, int en)
 	default:
 		printk(KERN_ERR "dm355_enable_clock:"
 				" Invalid selector: %d\n", clock_sel);
-		return -1;
+		return -EINVAL;
 	}
 
 	spin_lock_irqsave(&oper_cfg.vpss_lock, flags);
@@ -181,100 +241,221 @@ static int dm355_enable_clock(enum vpss_clock_sel clock_sel, int en)
 	return 0;
 }
 
+static int dm365_enable_clock(enum vpss_clock_sel clock_sel, int en)
+{
+	unsigned long flags;
+	u32 utemp, mask = 0x1, shift = 0, offset = DM365_ISP5_PCCR;
+	u32 (*read)(u32 offset) = isp5_read;
+	void(*write)(u32 val, u32 offset) = isp5_write;
+
+	switch (clock_sel) {
+	case VPSS_BL_CLOCK:
+		break;
+	case VPSS_CCDC_CLOCK:
+		shift = 1;
+		break;
+	case VPSS_H3A_CLOCK:
+		shift = 2;
+		break;
+	case VPSS_RSZ_CLOCK:
+		shift = 3;
+		break;
+	case VPSS_IPIPE_CLOCK:
+		shift = 4;
+		break;
+	case VPSS_IPIPEIF_CLOCK:
+		shift = 5;
+		break;
+	case VPSS_PCLK_INTERNAL:
+		shift = 6;
+		break;
+	case VPSS_PSYNC_CLOCK_SEL:
+		shift = 7;
+		break;
+	case VPSS_VPBE_CLOCK:
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	case VPSS_VENC_CLOCK_SEL:
+		shift = 2;
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	case VPSS_LDC_CLOCK:
+		shift = 3;
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	case VPSS_FDIF_CLOCK:
+		shift = 4;
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	case VPSS_OSD_CLOCK_SEL:
+		shift = 6;
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	case VPSS_LDC_CLOCK_SEL:
+		shift = 7;
+		read = vpss_regr;
+		write = vpss_regw;
+		offset = DM365_VPBE_CLK_CTRL;
+		break;
+	default:
+		printk(KERN_ERR "dm365_enable_clock: Invalid selector: %d\n",
+		       clock_sel);
+		return -1;
+	}
+
+	spin_lock_irqsave(&oper_cfg.vpss_lock, flags);
+	utemp = read(offset);
+	if (!en) {
+		mask = ~mask;
+		utemp &= (mask << shift);
+	} else
+		utemp |= (mask << shift);
+
+	write(utemp, offset);
+	spin_unlock_irqrestore(&oper_cfg.vpss_lock, flags);
+
+	return 0;
+}
+
 int vpss_enable_clock(enum vpss_clock_sel clock_sel, int en)
 {
 	if (!oper_cfg.hw_ops.enable_clock)
-		return -1;
+		return -EINVAL;
 
 	return oper_cfg.hw_ops.enable_clock(clock_sel, en);
 }
 EXPORT_SYMBOL(vpss_enable_clock);
 
+void dm365_vpss_set_sync_pol(struct vpss_sync_pol sync)
+{
+	int val = 0;
+	val = isp5_read(DM365_ISP5_CCDCMUX);
+
+	val |= (sync.ccdpg_hdpol << DM365_CCDC_PG_HD_POL_SHIFT);
+	val |= (sync.ccdpg_vdpol << DM365_CCDC_PG_VD_POL_SHIFT);
+
+	isp5_write(val, DM365_ISP5_CCDCMUX);
+}
+EXPORT_SYMBOL(dm365_vpss_set_sync_pol);
+
+void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size frame_size)
+{
+	int current_reg = ((frame_size.hlpfr >> 1) - 1) << 16;
+
+	current_reg |= (frame_size.pplen - 1);
+	isp5_write(current_reg, DM365_ISP5_PG_FRAME_SIZE);
+}
+EXPORT_SYMBOL(dm365_vpss_set_pg_frame_size);
+
 static int __init vpss_probe(struct platform_device *pdev)
 {
-	int status, dm355 = 0;
+	struct resource		*r1, *r2;
+	char *platform_name;
+	int status;
 
 	if (!pdev->dev.platform_data) {
 		dev_err(&pdev->dev, "no platform data\n");
 		return -ENOENT;
 	}
-	strcpy(oper_cfg.vpss_name, pdev->dev.platform_data);
 
-	if (!strcmp(oper_cfg.vpss_name, "dm355_vpss"))
-		dm355 = 1;
-	else if (strcmp(oper_cfg.vpss_name, "dm644x_vpss")) {
+	platform_name = pdev->dev.platform_data;
+	if (!strcmp(platform_name, "dm355_vpss"))
+		oper_cfg.platform = DM355;
+	else if (!strcmp(platform_name, "dm365_vpss"))
+		oper_cfg.platform = DM365;
+	else if (!strcmp(platform_name, "dm644x_vpss"))
+		oper_cfg.platform = DM644X;
+	else {
 		dev_err(&pdev->dev, "vpss driver not supported on"
 			" this platform\n");
 		return -ENODEV;
 	}
 
-	dev_info(&pdev->dev, "%s vpss probed\n", oper_cfg.vpss_name);
-	oper_cfg.r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!oper_cfg.r1)
+	dev_info(&pdev->dev, "%s vpss probed\n", platform_name);
+	r1 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r1)
 		return -ENOENT;
 
-	oper_cfg.len1 = oper_cfg.r1->end - oper_cfg.r1->start + 1;
-
-	oper_cfg.r1 = request_mem_region(oper_cfg.r1->start, oper_cfg.len1,
-					 oper_cfg.r1->name);
-	if (!oper_cfg.r1)
+	r1 = request_mem_region(r1->start, resource_size(r1), r1->name);
+	if (!r1)
 		return -EBUSY;
 
-	oper_cfg.vpss_bl_regs_base = ioremap(oper_cfg.r1->start, oper_cfg.len1);
-	if (!oper_cfg.vpss_bl_regs_base) {
+	oper_cfg.vpss_regs_base0 = ioremap(r1->start, resource_size(r1));
+	if (!oper_cfg.vpss_regs_base0) {
 		status = -EBUSY;
 		goto fail1;
 	}
 
-	if (dm355) {
-		oper_cfg.r2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
-		if (!oper_cfg.r2) {
+	if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) {
+		r2 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		if (!r2) {
 			status = -ENOENT;
 			goto fail2;
 		}
-		oper_cfg.len2 = oper_cfg.r2->end - oper_cfg.r2->start + 1;
-		oper_cfg.r2 = request_mem_region(oper_cfg.r2->start,
-						 oper_cfg.len2,
-						 oper_cfg.r2->name);
-		if (!oper_cfg.r2) {
+		r2 = request_mem_region(r2->start, resource_size(r2), r2->name);
+		if (!r2) {
 			status = -EBUSY;
 			goto fail2;
 		}
 
-		oper_cfg.vpss_regs_base = ioremap(oper_cfg.r2->start,
-						  oper_cfg.len2);
-		if (!oper_cfg.vpss_regs_base) {
+		oper_cfg.vpss_regs_base1 = ioremap(r2->start,
+						   resource_size(r2));
+		if (!oper_cfg.vpss_regs_base1) {
 			status = -EBUSY;
 			goto fail3;
 		}
 	}
 
-	if (dm355) {
+	if (oper_cfg.platform == DM355) {
 		oper_cfg.hw_ops.enable_clock = dm355_enable_clock;
 		oper_cfg.hw_ops.select_ccdc_source = dm355_select_ccdc_source;
+		/* Setup vpss interrupts */
+		bl_regw(DM355_VPSSBL_INTSEL_DEFAULT, DM355_VPSSBL_INTSEL);
+		bl_regw(DM355_VPSSBL_EVTSEL_DEFAULT, DM355_VPSSBL_EVTSEL);
+	} else if (oper_cfg.platform == DM365) {
+		oper_cfg.hw_ops.enable_clock = dm365_enable_clock;
+		oper_cfg.hw_ops.select_ccdc_source = dm365_select_ccdc_source;
+		/* Setup vpss interrupts */
+		isp5_write(DM365_ISP5_INTSEL1_DEFAULT, DM365_ISP5_INTSEL1);
+		isp5_write(DM365_ISP5_INTSEL2_DEFAULT, DM365_ISP5_INTSEL2);
+		isp5_write(DM365_ISP5_INTSEL3_DEFAULT, DM365_ISP5_INTSEL3);
 	} else
 		oper_cfg.hw_ops.clear_wbl_overflow = dm644x_clear_wbl_overflow;
 
 	spin_lock_init(&oper_cfg.vpss_lock);
-	dev_info(&pdev->dev, "%s vpss probe success\n", oper_cfg.vpss_name);
+	dev_info(&pdev->dev, "%s vpss probe success\n", platform_name);
 	return 0;
 
 fail3:
-	release_mem_region(oper_cfg.r2->start, oper_cfg.len2);
+	release_mem_region(r2->start, resource_size(r2));
 fail2:
-	iounmap(oper_cfg.vpss_bl_regs_base);
+	iounmap(oper_cfg.vpss_regs_base0);
 fail1:
-	release_mem_region(oper_cfg.r1->start, oper_cfg.len1);
+	release_mem_region(r1->start, resource_size(r1));
 	return status;
 }
 
 static int vpss_remove(struct platform_device *pdev)
 {
-	iounmap(oper_cfg.vpss_bl_regs_base);
-	release_mem_region(oper_cfg.r1->start, oper_cfg.len1);
-	if (!strcmp(oper_cfg.vpss_name, "dm355_vpss")) {
-		iounmap(oper_cfg.vpss_regs_base);
-		release_mem_region(oper_cfg.r2->start, oper_cfg.len2);
+	struct resource		*res;
+
+	iounmap(oper_cfg.vpss_regs_base0);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(res->start, resource_size(res));
+	if (oper_cfg.platform == DM355 || oper_cfg.platform == DM365) {
+		iounmap(oper_cfg.vpss_regs_base1);
+		res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+		release_mem_region(res->start, resource_size(res));
 	}
 	return 0;
 }
diff --git a/include/media/davinci/vpss.h b/include/media/davinci/vpss.h
index fcdff74..c59cc02 100644
--- a/include/media/davinci/vpss.h
+++ b/include/media/davinci/vpss.h
@@ -29,7 +29,19 @@
 /* selector for ccdc input selection on DM355 */
 enum vpss_ccdc_source_sel {
 	VPSS_CCDCIN,
-	VPSS_HSSIIN
+	VPSS_HSSIIN,
+	VPSS_PGLPBK,	/* for DM365 only */
+	VPSS_CCDCPG	/* for DM365 only */
+};
+
+struct vpss_sync_pol {
+	unsigned int ccdpg_hdpol:1;
+	unsigned int ccdpg_vdpol:1;
+};
+
+struct vpss_pg_frame_size {
+	short hlpfr;
+	short pplen;
 };
 
 /* Used for enable/diable VPSS Clock */
@@ -47,12 +59,38 @@ enum vpss_clock_sel {
 	 */
 	VPSS_VENC_CLOCK_SEL,
 	VPSS_VPBE_CLOCK,
+	/* DM365 only clocks */
+	VPSS_IPIPEIF_CLOCK,
+	VPSS_RSZ_CLOCK,
+	VPSS_BL_CLOCK,
+	/*
+	 * When using VPSS_PCLK_INTERNAL in vpss_enable_clock() api
+	 * following applies:-
+	 * en = 0 disable internal PCLK
+	 * en = 1 enables internal PCLK
+	 */
+	VPSS_PCLK_INTERNAL,
+	/*
+	 * When using VPSS_PSYNC_CLOCK_SEL in vpss_enable_clock() api
+	 * following applies:-
+	 * en = 0 enables MMR clock
+	 * en = 1 enables VPSS clock
+	 */
+	VPSS_PSYNC_CLOCK_SEL,
+	VPSS_LDC_CLOCK_SEL,
+	VPSS_OSD_CLOCK_SEL,
+	VPSS_FDIF_CLOCK,
+	VPSS_LDC_CLOCK
 };
 
 /* select input to ccdc on dm355 */
 int vpss_select_ccdc_source(enum vpss_ccdc_source_sel src_sel);
 /* enable/disable a vpss clock, 0 - success, -1 - failure */
 int vpss_enable_clock(enum vpss_clock_sel clock_sel, int en);
+/* set sync polarity, only for DM365*/
+void dm365_vpss_set_sync_pol(struct vpss_sync_pol);
+/* set the PG_FRAME_SIZE register, only for DM365 */
+void dm365_vpss_set_pg_frame_size(struct vpss_pg_frame_size);
 
 /* wbl reset for dm644x */
 enum vpss_wbl_sel {
@@ -65,5 +103,6 @@ enum vpss_wbl_sel {
 	VPSS_PCR_PREV_WBL_0,
 	VPSS_PCR_CCDC_WBL_O,
 };
+/* clear wbl overflow flag for DM6446 */
 int vpss_clear_wbl_overflow(enum vpss_wbl_sel wbl_sel);
 #endif
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver
  2009-12-10 17:00     ` [PATCH - v1 1/6] V4L - vpfe-capture : DM365 vpss enhancements m-karicheri2
@ 2009-12-10 17:00       ` m-karicheri2
  2009-12-10 17:00         ` [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements m-karicheri2
  2009-12-10 17:38         ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver Sergei Shtylyov
  0 siblings, 2 replies; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

Adding Makefile and Kconfig for ISIF driver

Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
Applies to linux-next tree
 drivers/media/video/Kconfig          |   15 ++++++++++++++-
 drivers/media/video/davinci/Makefile |    1 +
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
index 9dc74c9..8250c68 100644
--- a/drivers/media/video/Kconfig
+++ b/drivers/media/video/Kconfig
@@ -552,7 +552,7 @@ config VIDEO_VPSS_SYSTEM
 	depends on ARCH_DAVINCI
 	help
 	  Support for vpss system module for video driver
-	default y
+	default n
 
 config VIDEO_VPFE_CAPTURE
 	tristate "VPFE Video Capture Driver"
@@ -596,6 +596,19 @@ config VIDEO_DM355_CCDC
 	   To compile this driver as a module, choose M here: the
 	   module will be called vpfe.
 
+config VIDEO_ISIF
+	tristate "ISIF HW module"
+	depends on ARCH_DAVINCI_DM365 && VIDEO_VPFE_CAPTURE
+	select VIDEO_VPSS_SYSTEM
+	default y
+	help
+	   Enables ISIF hw module. This is the hardware module for
+	   configuring ISIF in VPFE to capture Raw Bayer RGB data  from
+	   a image sensor or YUV data from a YUV source.
+
+	   To compile this driver as a module, choose M here: the
+	   module will be called vpfe.
+
 source "drivers/media/video/bt8xx/Kconfig"
 
 config VIDEO_PMS
diff --git a/drivers/media/video/davinci/Makefile b/drivers/media/video/davinci/Makefile
index 1a8b8f3..a379557 100644
--- a/drivers/media/video/davinci/Makefile
+++ b/drivers/media/video/davinci/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_VIDEO_VPSS_SYSTEM) += vpss.o
 obj-$(CONFIG_VIDEO_VPFE_CAPTURE) += vpfe_capture.o
 obj-$(CONFIG_VIDEO_DM6446_CCDC) += dm644x_ccdc.o
 obj-$(CONFIG_VIDEO_DM355_CCDC) += dm355_ccdc.o
+obj-$(CONFIG_VIDEO_ISIF) += isif.o
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-10 17:00       ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver m-karicheri2
@ 2009-12-10 17:00         ` m-karicheri2
  2009-12-15 21:05           ` Hans Verkuil
  2009-12-10 17:38         ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver Sergei Shtylyov
  1 sibling, 1 reply; 17+ messages in thread
From: m-karicheri2 @ 2009-12-10 17:00 UTC (permalink / raw)
  To: linux-media, hverkuil, khilman, nsekhar, hvaibhav
  Cc: davinci-linux-open-source, Muralidharan Karicheri

From: Muralidharan Karicheri <m-karicheri2@ti.com>

Added a experimental IOCTL, to read the CCDC parameters.
Default handler was not getting the original pointer from the core.
So a wrapper function added to handle the default handler properly.
Also fixed a bug in the probe() in the linux-next tree

Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
---
 drivers/media/video/davinci/vpfe_capture.c |  119 +++++++++++++++++-----------
 include/media/davinci/vpfe_capture.h       |   12 ++-
 2 files changed, 81 insertions(+), 50 deletions(-)

diff --git a/drivers/media/video/davinci/vpfe_capture.c b/drivers/media/video/davinci/vpfe_capture.c
index 091750e..8c6d856 100644
--- a/drivers/media/video/davinci/vpfe_capture.c
+++ b/drivers/media/video/davinci/vpfe_capture.c
@@ -759,12 +759,83 @@ static unsigned int vpfe_poll(struct file *file, poll_table *wait)
 	return 0;
 }
 
+static long vpfe_param_handler(struct file *file, void *priv,
+		int cmd, void *param)
+{
+	struct vpfe_device *vpfe_dev = video_drvdata(file);
+	int ret = 0;
+
+	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_param_handler\n");
+
+	if (NULL == param) {
+		v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
+			"Invalid user ptr\n");
+	}
+
+	if (vpfe_dev->started) {
+		/* only allowed if streaming is not started */
+		v4l2_err(&vpfe_dev->v4l2_dev, "device already started\n");
+		return -EBUSY;
+	}
+
+
+	switch (cmd) {
+	case VPFE_CMD_S_CCDC_RAW_PARAMS:
+		v4l2_warn(&vpfe_dev->v4l2_dev,
+			  "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
+		ret = mutex_lock_interruptible(&vpfe_dev->lock);
+		if (ret)
+			return ret;
+		ret = ccdc_dev->hw_ops.set_params(param);
+		if (ret) {
+			v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
+				"Error in setting parameters in CCDC\n");
+			goto unlock_out;
+		}
+
+		if (vpfe_get_ccdc_image_format(vpfe_dev, &vpfe_dev->fmt) < 0) {
+			v4l2_err(&vpfe_dev->v4l2_dev,
+				"Invalid image format at CCDC\n");
+			ret = -EINVAL;
+		}
+unlock_out:
+		mutex_unlock(&vpfe_dev->lock);
+		break;
+	case VPFE_CMD_G_CCDC_RAW_PARAMS:
+		v4l2_warn(&vpfe_dev->v4l2_dev,
+			  "VPFE_CMD_G_CCDC_RAW_PARAMS: experimental ioctl\n");
+		if (!ccdc_dev->hw_ops.get_params) {
+			ret = -EINVAL;
+			break;
+		}
+		ret = ccdc_dev->hw_ops.get_params(param);
+		if (ret) {
+			v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
+				"Error in getting parameters from CCDC\n");
+		}
+		break;
+
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static long vpfe_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	if (cmd == VPFE_CMD_S_CCDC_RAW_PARAMS ||
+	    cmd == VPFE_CMD_G_CCDC_RAW_PARAMS)
+		return vpfe_param_handler(file, file->private_data, cmd,
+					 (void *)arg);
+	return video_ioctl2(file, cmd, arg);
+}
+
 /* vpfe capture driver file operations */
 static const struct v4l2_file_operations vpfe_fops = {
 	.owner = THIS_MODULE,
 	.open = vpfe_open,
 	.release = vpfe_release,
-	.unlocked_ioctl = video_ioctl2,
+	.unlocked_ioctl = vpfe_ioctl,
 	.mmap = vpfe_mmap,
 	.poll = vpfe_poll
 };
@@ -1682,50 +1753,6 @@ unlock_out:
 	return ret;
 }
 
-
-static long vpfe_param_handler(struct file *file, void *priv,
-		int cmd, void *param)
-{
-	struct vpfe_device *vpfe_dev = video_drvdata(file);
-	int ret = 0;
-
-	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_param_handler\n");
-
-	if (vpfe_dev->started) {
-		/* only allowed if streaming is not started */
-		v4l2_err(&vpfe_dev->v4l2_dev, "device already started\n");
-		return -EBUSY;
-	}
-
-	ret = mutex_lock_interruptible(&vpfe_dev->lock);
-	if (ret)
-		return ret;
-
-	switch (cmd) {
-	case VPFE_CMD_S_CCDC_RAW_PARAMS:
-		v4l2_warn(&vpfe_dev->v4l2_dev,
-			  "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
-		ret = ccdc_dev->hw_ops.set_params(param);
-		if (ret) {
-			v4l2_err(&vpfe_dev->v4l2_dev,
-				"Error in setting parameters in CCDC\n");
-			goto unlock_out;
-		}
-		if (vpfe_get_ccdc_image_format(vpfe_dev, &vpfe_dev->fmt) < 0) {
-			v4l2_err(&vpfe_dev->v4l2_dev,
-				"Invalid image format at CCDC\n");
-			goto unlock_out;
-		}
-		break;
-	default:
-		ret = -EINVAL;
-	}
-unlock_out:
-	mutex_unlock(&vpfe_dev->lock);
-	return ret;
-}
-
-
 /* vpfe capture ioctl operations */
 static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
 	.vidioc_querycap	 = vpfe_querycap,
@@ -1751,7 +1778,6 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
 	.vidioc_cropcap		 = vpfe_cropcap,
 	.vidioc_g_crop		 = vpfe_g_crop,
 	.vidioc_s_crop		 = vpfe_s_crop,
-	.vidioc_default		 = vpfe_param_handler,
 };
 
 static struct vpfe_device *vpfe_initialize(void)
@@ -1923,6 +1949,7 @@ static __init int vpfe_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, vpfe_dev);
 	/* set driver private data */
 	video_set_drvdata(vpfe_dev->video_dev, vpfe_dev);
+	vpfe_cfg = pdev->dev.platform_data;
 	i2c_adap = i2c_get_adapter(vpfe_cfg->i2c_adapter_id);
 	num_subdevs = vpfe_cfg->num_subdevs;
 	vpfe_dev->sd = kmalloc(sizeof(struct v4l2_subdev *) * num_subdevs,
diff --git a/include/media/davinci/vpfe_capture.h b/include/media/davinci/vpfe_capture.h
index d863e5e..23043bd 100644
--- a/include/media/davinci/vpfe_capture.h
+++ b/include/media/davinci/vpfe_capture.h
@@ -31,8 +31,6 @@
 #include <media/videobuf-dma-contig.h>
 #include <media/davinci/vpfe_types.h>
 
-#define VPFE_CAPTURE_NUM_DECODERS        5
-
 /* Macros */
 #define VPFE_MAJOR_RELEASE              0
 #define VPFE_MINOR_RELEASE              0
@@ -91,8 +89,9 @@ struct vpfe_config {
 	char *card_name;
 	/* ccdc name */
 	char *ccdc;
-	/* vpfe clock */
+	/* vpfe clock. Obsolete! Will be removed in next patch */
 	struct clk *vpssclk;
+	/* Obsolete! Will be removed in next patch */
 	struct clk *slaveclk;
 };
 
@@ -193,8 +192,13 @@ struct vpfe_config_params {
  * color space conversion, culling etc. This is an experimental ioctl that
  * will change in future kernels. So use this ioctl with care !
  * TODO: This is to be split into multiple ioctls and also explore the
- * possibility of extending the v4l2 api to include this
+ * possibility of extending the v4l2 api to include this.
+ * VPFE_CMD_G_CCDC_RAW_PARAMS - EXPERIMENTAL IOCTL to get the current raw
+ * capture parameters
  **/
 #define VPFE_CMD_S_CCDC_RAW_PARAMS _IOW('V', BASE_VIDIOC_PRIVATE + 1, \
 					void *)
+#define VPFE_CMD_G_CCDC_RAW_PARAMS _IOR('V', BASE_VIDIOC_PRIVATE + 2, \
+					void *)
+
 #endif				/* _DAVINCI_VPFE_H */
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver
  2009-12-10 17:00       ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver m-karicheri2
  2009-12-10 17:00         ` [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements m-karicheri2
@ 2009-12-10 17:38         ` Sergei Shtylyov
  2009-12-11 20:50           ` Karicheri, Muralidharan
  1 sibling, 1 reply; 17+ messages in thread
From: Sergei Shtylyov @ 2009-12-10 17:38 UTC (permalink / raw)
  To: m-karicheri2
  Cc: linux-media, hverkuil, khilman, nsekhar, hvaibhav,
	davinci-linux-open-source

Hello.

m-karicheri2@ti.com wrote:

> From: Muralidharan Karicheri <m-karicheri2@ti.com>
>
> Adding Makefile and Kconfig for ISIF driver
>
> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
> ---
> Applies to linux-next tree
>  drivers/media/video/Kconfig          |   15 ++++++++++++++-
>  drivers/media/video/davinci/Makefile |    1 +
>  2 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index 9dc74c9..8250c68 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -552,7 +552,7 @@ config VIDEO_VPSS_SYSTEM
>  	depends on ARCH_DAVINCI
>  	help
>  	  Support for vpss system module for video driver
> -	default y
> +	default n
>   

   You might as well have deleted "default".

WBR, Sergei


^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver
  2009-12-10 17:38         ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver Sergei Shtylyov
@ 2009-12-11 20:50           ` Karicheri, Muralidharan
  0 siblings, 0 replies; 17+ messages in thread
From: Karicheri, Muralidharan @ 2009-12-11 20:50 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-media, hverkuil, khilman, Nori, Sekhar, Hiremath, Vaibhav,
	davinci-linux-open-source

Ok. I will do.

Murali Karicheri
Software Design Engineer
Texas Instruments Inc.
Germantown, MD 20874
phone: 301-407-9583
email: m-karicheri2@ti.com

>-----Original Message-----
>From: Sergei Shtylyov [mailto:sshtylyov@ru.mvista.com]
>Sent: Thursday, December 10, 2009 12:38 PM
>To: Karicheri, Muralidharan
>Cc: linux-media@vger.kernel.org; hverkuil@xs4all.nl;
>khilman@deeprootsystems.com; Nori, Sekhar; Hiremath, Vaibhav; davinci-
>linux-open-source@linux.davincidsp.com
>Subject: Re: [PATCH - v1 5/6] V4L - vpfe capture - build environment for
>ISIF driver
>
>Hello.
>
>m-karicheri2@ti.com wrote:
>
>> From: Muralidharan Karicheri <m-karicheri2@ti.com>
>>
>> Adding Makefile and Kconfig for ISIF driver
>>
>> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
>> ---
>> Applies to linux-next tree
>>  drivers/media/video/Kconfig          |   15 ++++++++++++++-
>>  drivers/media/video/davinci/Makefile |    1 +
>>  2 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
>> index 9dc74c9..8250c68 100644
>> --- a/drivers/media/video/Kconfig
>> +++ b/drivers/media/video/Kconfig
>> @@ -552,7 +552,7 @@ config VIDEO_VPSS_SYSTEM
>>  	depends on ARCH_DAVINCI
>>  	help
>>  	  Support for vpss system module for video driver
>> -	default y
>> +	default n
>>
>
>   You might as well have deleted "default".
>
>WBR, Sergei


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files
  2009-12-10 17:00 ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files m-karicheri2
  2009-12-10 17:00   ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source m-karicheri2
@ 2009-12-15  7:46   ` Hans Verkuil
  2009-12-17 21:34     ` Karicheri, Muralidharan
  1 sibling, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2009-12-15  7:46 UTC (permalink / raw)
  To: m-karicheri2
  Cc: linux-media, khilman, nsekhar, hvaibhav, davinci-linux-open-source

Hi Murali,

Here is the first review. I'll try to review all the patches from this patch series
this week. FYI, patch 1/6 is fine.

On Thursday 10 December 2009 18:00:25 m-karicheri2@ti.com wrote:
> From: Muralidharan Karicheri <m-karicheri2@ti.com>
> 
> This is the header file for ISIF driver on DM365. This has comments incorporated from
> initial version.
> 
> ISIF driver is equivalent to CCDC driver on DM355 and DM644x. This driver is tested for
> YUV capture from TVP514x driver. This patch contains the header files required for
> this driver. The name of the file is changed to reflect the name of IP.
> 
> Reviewed-by: Nori, Sekhar <nsekhar@ti.com>
> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
> ---
> Applies to linux-next tree of v4l-dvb 
>  drivers/media/video/davinci/isif_regs.h |  290 ++++++++++++++++
>  include/media/davinci/isif.h            |  554 +++++++++++++++++++++++++++++++
>  2 files changed, 844 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/davinci/isif_regs.h
>  create mode 100644 include/media/davinci/isif.h
> 

<cut>

> diff --git a/include/media/davinci/isif.h b/include/media/davinci/isif.h
> new file mode 100644
> index 0000000..b75fea7
> --- /dev/null
> +++ b/include/media/davinci/isif.h
> @@ -0,0 +1,554 @@
> +/*
> + * Copyright (C) 2008-2009 Texas Instruments Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * isif header file
> + */
> +#ifndef _ISIF_H
> +#define _ISIF_H

Please add an empty line.

> +#include <media/davinci/ccdc_types.h>
> +#include <media/davinci/vpfe_types.h>

Ditto.

> +/* isif float type S8Q8/U8Q8 */
> +struct isif_float_8 {
> +	/* 8 bit integer part */
> +	__u8 integer;
> +	/* 8 bit decimal part */
> +	__u8 decimal;

Why __u8 instead of u8? This is a kernel-internal file, so it does not need
to use the '__' variants.

If this needs to be publicly available, then it should be in include/linux.

> +};
> +
> +/* isif float type U16Q16/S16Q16 */
> +struct isif_float_16 {
> +	/* 16 bit integer part */
> +	__u16 integer;
> +	/* 16 bit decimal part */
> +	__u16 decimal;
> +};
> +
> +/************************************************************************
> + *   Vertical Defect Correction parameters
> + ***********************************************************************/
> +/* Defect Correction (DFC) table entry */
> +struct isif_vdfc_entry {
> +	/* vertical position of defect */
> +	__u16 pos_vert;
> +	/* horizontal position of defect */
> +	__u16 pos_horz;
> +	/*
> +	 * Defect level of Vertical line defect position. This is subtracted
> +	 * from the data at the defect position
> +	 */
> +	__u8 level_at_pos;
> +	/*
> +	 * Defect level of the pixels upper than the vertical line defect.
> +	 * This is subtracted from the data
> +	 */
> +	__u8 level_up_pixels;
> +	/*
> +	 * Defect level of the pixels lower than the vertical line defect.
> +	 * This is subtracted from the data
> +	 */
> +	__u8 level_low_pixels;
> +};
> +
> +#define ISIF_VDFC_TABLE_SIZE		8
> +struct isif_dfc {
> +	/* enable vertical defect correction */
> +	__u8 en;
> +	/* Defect level subtraction. Just fed through if saturating */
> +#define	ISIF_VDFC_NORMAL		0
> +	/*
> +	 * Defect level subtraction. Horizontal interpolation ((i-2)+(i+2))/2
> +	 * if data saturating
> +	 */
> +#define ISIF_VDFC_HORZ_INTERPOL_IF_SAT	1
> +	/* Horizontal interpolation (((i-2)+(i+2))/2) */
> +#define	ISIF_VDFC_HORZ_INTERPOL		2
> +	/* one of the vertical defect correction modes above */
> +	__u8 corr_mode;
> +	/* 0 - whole line corrected, 1 - not pixels upper than the defect */
> +	__u8 corr_whole_line;
> +#define ISIF_VDFC_NO_SHIFT		0
> +#define ISIF_VDFC_SHIFT_1		1
> +#define ISIF_VDFC_SHIFT_2		2
> +#define ISIF_VDFC_SHIFT_3		3
> +#define ISIF_VDFC_SHIFT_4		4
> +	/*
> +	 * defect level shift value. level_at_pos, level_upper_pos,
> +	 * and level_lower_pos can be shifted up by this value. Choose
> +	 * one of the values above
> +	 */
> +	__u8 def_level_shift;
> +	/* defect saturation level */
> +	__u16 def_sat_level;
> +	/* number of vertical defects. Max is ISIF_VDFC_TABLE_SIZE */
> +	__u16 num_vdefects;
> +	/* VDFC table ptr */
> +	struct isif_vdfc_entry table[ISIF_VDFC_TABLE_SIZE];
> +};
> +
> +struct isif_horz_bclamp {
> +
> +	/* Horizontal clamp disabled. Only vertical clamp value is subtracted */
> +#define	ISIF_HORZ_BC_DISABLE		0
> +	/*
> +	 * Horizontal clamp value is calculated and subtracted from image data
> +	 * along with vertical clamp value
> +	 */
> +#define ISIF_HORZ_BC_CLAMP_CALC_ENABLED	1
> +	/*
> +	 * Horizontal clamp value calculated from previous image is subtracted
> +	 * from image data along with vertical clamp value.
> +	 */
> +#define ISIF_HORZ_BC_CLAMP_NOT_UPDATED	2
> +	/* horizontal clamp mode. One of the values above */
> +	__u8 mode;
> +	/*
> +	 * pixel value limit enable.
> +	 *  0 - limit disabled
> +	 *  1 - pixel value limited to 1023
> +	 */
> +	__u8 clamp_pix_limit;
> +	/* Select Most left window for bc calculation */
> +#define	ISIF_SEL_MOST_LEFT_WIN		0
> +	/* Select Most right window for bc calculation */
> +#define ISIF_SEL_MOST_RIGHT_WIN		1
> +	/* Select most left or right window for clamp val calculation */
> +	__u8 base_win_sel_calc;
> +	/* Window count per color for calculation. range 1-32 */
> +	__u8 win_count_calc;
> +	/* Window start position - horizontal for calculation. 0 - 8191 */
> +	__u16 win_start_h_calc;
> +	/* Window start position - vertical for calculation 0 - 8191 */
> +	__u16 win_start_v_calc;
> +#define ISIF_HORZ_BC_SZ_H_2PIXELS	0
> +#define ISIF_HORZ_BC_SZ_H_4PIXELS	1
> +#define ISIF_HORZ_BC_SZ_H_8PIXELS	2
> +#define ISIF_HORZ_BC_SZ_H_16PIXELS	3
> +	/* Width of the sample window in pixels for calculation */
> +	__u8 win_h_sz_calc;
> +#define ISIF_HORZ_BC_SZ_V_32PIXELS	0
> +#define ISIF_HORZ_BC_SZ_V_64PIXELS	1
> +#define	ISIF_HORZ_BC_SZ_V_128PIXELS	2
> +#define ISIF_HORZ_BC_SZ_V_256PIXELS	3
> +	/* Height of the sample window in pixels for calculation */
> +	__u8 win_v_sz_calc;
> +};
> +
> +/************************************************************************
> + *  Black Clamp parameters
> + ***********************************************************************/
> +struct isif_vert_bclamp {
> +	/* Reset value used is the clamp value calculated */
> +#define	ISIF_VERT_BC_USE_HORZ_CLAMP_VAL		0
> +	/* Reset value used is reset_clamp_val configured */
> +#define	ISIF_VERT_BC_USE_CONFIG_CLAMP_VAL	1
> +	/* No update, previous image value is used */
> +#define	ISIF_VERT_BC_NO_UPDATE			2
> +	/*
> +	 * Reset value selector for vertical clamp calculation. Use one of
> +	 * the above values
> +	 */
> +	__u8 reset_val_sel;
> +	/* U12 value if reset_val_sel = ISIF_BC_VERT_USE_CONFIG_CLAMP_VAL */
> +	__u16 reset_clamp_val;
> +	/* U8Q8. Line average coefficient used in vertical clamp calculation */
> +	__u8 line_ave_coef;
> +#define	ISIF_VERT_BC_SZ_H_2PIXELS	0
> +#define ISIF_VERT_BC_SZ_H_4PIXELS	1
> +#define	ISIF_VERT_BC_SZ_H_8PIXELS	2
> +#define	ISIF_VERT_BC_SZ_H_16PIXELS	3
> +#define	ISIF_VERT_BC_SZ_H_32PIXELS	4
> +#define	ISIF_VERT_BC_SZ_H_64PIXELS	5
> +	/* Width in pixels of the optical black region used for calculation */
> +	__u8 ob_h_sz_calc;
> +	/* Height of the optical black region for calculation */
> +	__u16 ob_v_sz_calc;
> +	/* Optical black region start position - horizontal. 0 - 8191 */
> +	__u16 ob_start_h;
> +	/* Optical black region start position - vertical 0 - 8191 */
> +	__u16 ob_start_v;
> +};
> +
> +struct isif_black_clamp {
> +	/*
> +	 * This offset value is added irrespective of the clamp enable status.
> +	 * S13
> +	 */
> +	__u16 dc_offset;
> +	/*
> +	 * Enable black/digital clamp value to be subtracted from the image data
> +	 */
> +	__u8 en;
> +	/*
> +	 * black clamp mode. same/separate clamp for 4 colors
> +	 * 0 - disable - same clamp value for all colors
> +	 * 1 - clamp value calculated separately for all colors
> +	 */
> +	__u8 bc_mode_color;
> +	/* Vrtical start position for bc subtraction */
> +	__u16 vert_start_sub;
> +	/* Black clamp for horizontal direction */
> +	struct isif_horz_bclamp horz;
> +	/* Black clamp for vertical direction */
> +	struct isif_vert_bclamp vert;
> +};
> +
> +/*************************************************************************
> +** Color Space Convertion (CSC)
> +*************************************************************************/
> +#define ISIF_CSC_NUM_COEFF	16
> +struct isif_color_space_conv {
> +	/* Enable color space conversion */
> +	__u8 en;
> +	/*
> +	 * csc coeffient table. S8Q5, M00 at index 0, M01 at index 1, and
> +	 * so forth
> +	 */
> +	struct isif_float_8 coeff[ISIF_CSC_NUM_COEFF];
> +};
> +
> +
> +/*************************************************************************
> +**  Black  Compensation parameters
> +*************************************************************************/
> +struct isif_black_comp {
> +	/* Comp for Red */
> +	__s8 r_comp;
> +	/* Comp for Gr */
> +	__s8 gr_comp;
> +	/* Comp for Blue */
> +	__s8 b_comp;
> +	/* Comp for Gb */
> +	__s8 gb_comp;
> +};
> +
> +/*************************************************************************
> +**  Gain parameters
> +*************************************************************************/
> +struct isif_gain {
> +	/* Gain for Red or ye */
> +	struct isif_float_16 r_ye;
> +	/* Gain for Gr or cy */
> +	struct isif_float_16 gr_cy;
> +	/* Gain for Gb or g */
> +	struct isif_float_16 gb_g;
> +	/* Gain for Blue or mg */
> +	struct isif_float_16 b_mg;
> +};
> +
> +#define ISIF_LINEAR_TAB_SIZE	192
> +/*************************************************************************
> +**  Linearization parameters
> +*************************************************************************/
> +struct isif_linearize {
> +	/* Enable or Disable linearization of data */
> +	__u8 en;
> +	/* Shift value applied */
> +	__u8 corr_shft;
> +	/* scale factor applied U11Q10 */
> +	struct isif_float_16 scale_fact;
> +	/* Size of the linear table */
> +	__u16 table[ISIF_LINEAR_TAB_SIZE];
> +};
> +
> +/* Color patterns */
> +#define ISIF_RED	0
> +#define	ISIF_GREEN_RED	1
> +#define ISIF_GREEN_BLUE	2
> +#define ISIF_BLUE	3
> +struct isif_col_pat {
> +	__u8 olop;
> +	__u8 olep;
> +	__u8 elop;
> +	__u8 elep;
> +};
> +
> +/*************************************************************************
> +**  Data formatter parameters
> +*************************************************************************/
> +struct isif_fmtplen {
> +	/*
> +	 * number of program entries for SET0, range 1 - 16
> +	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
> +	 * ISIF_COMBINE
> +	 */
> +	__u16 plen0;
> +	/*
> +	 * number of program entries for SET1, range 1 - 16
> +	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
> +	 * ISIF_COMBINE
> +	 */
> +	__u16 plen1;
> +	/**
> +	 * number of program entries for SET2, range 1 - 16
> +	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
> +	 * ISIF_COMBINE
> +	 */
> +	__u16 plen2;
> +	/**
> +	 * number of program entries for SET3, range 1 - 16
> +	 * when fmtmode is ISIF_SPLIT, 1 - 8 when fmtmode is
> +	 * ISIF_COMBINE
> +	 */
> +	__u16 plen3;
> +};
> +
> +struct isif_fmt_cfg {
> +#define ISIF_SPLIT		0
> +#define ISIF_COMBINE		1
> +	/* Split or combine or line alternate */
> +	__u8 fmtmode;
> +	/* enable or disable line alternating mode */
> +	__u8 ln_alter_en;
> +#define ISIF_1LINE		0
> +#define	ISIF_2LINES		1
> +#define	ISIF_3LINES		2
> +#define	ISIF_4LINES		3
> +	/* Split/combine line number */
> +	__u8 lnum;
> +	/* Address increment Range 1 - 16 */
> +	__u8 addrinc;
> +};
> +
> +struct isif_fmt_addr_ptr {
> +	/* Initial address */
> +	__u32 init_addr;
> +	/* output line number */
> +#define ISIF_1STLINE		0
> +#define	ISIF_2NDLINE		1
> +#define	ISIF_3RDLINE		2
> +#define	ISIF_4THLINE		3
> +	__u8 out_line;
> +};
> +
> +struct isif_fmtpgm_ap {
> +	/* program address pointer */
> +	__u8 pgm_aptr;
> +	/* program address increment or decrement */
> +	__u8 pgmupdt;
> +};
> +
> +struct isif_data_formatter {
> +	/* Enable/Disable data formatter */
> +	__u8 en;
> +	/* data formatter configuration */
> +	struct isif_fmt_cfg cfg;
> +	/* Formatter program entries length */
> +	struct isif_fmtplen plen;
> +	/* first pixel in a line fed to formatter */
> +	__u16 fmtrlen;
> +	/* HD interval for output line. Only valid when split line */
> +	__u16 fmthcnt;
> +	/* formatter address pointers */
> +	struct isif_fmt_addr_ptr fmtaddr_ptr[16];
> +	/* program enable/disable */
> +	__u8 pgm_en[32];
> +	/* program address pointers */
> +	struct isif_fmtpgm_ap fmtpgm_ap[32];
> +};
> +
> +struct isif_df_csc {
> +	/* Color Space Conversion confguration, 0 - csc, 1 - df */
> +	__u8 df_or_csc;
> +	/* csc configuration valid if df_or_csc is 0 */
> +	struct isif_color_space_conv csc;
> +	/* data formatter configuration valid if df_or_csc is 1 */
> +	struct isif_data_formatter df;
> +	/* start pixel in a line at the input */
> +	__u32 start_pix;
> +	/* number of pixels in input line */
> +	__u32 num_pixels;
> +	/* start line at the input */
> +	__u32 start_line;
> +	/* number of lines at the input */
> +	__u32 num_lines;
> +};
> +
> +struct isif_gain_offsets_adj {
> +	/* Gain adjustment per color */
> +	struct isif_gain gain;
> +	/* Offset adjustment */
> +	__u16 offset;
> +	/* Enable or Disable Gain adjustment for SDRAM data */
> +	__u8 gain_sdram_en;
> +	/* Enable or Disable Gain adjustment for IPIPE data */
> +	__u8 gain_ipipe_en;
> +	/* Enable or Disable Gain adjustment for H3A data */
> +	__u8 gain_h3a_en;
> +	/* Enable or Disable Gain adjustment for SDRAM data */
> +	__u8 offset_sdram_en;
> +	/* Enable or Disable Gain adjustment for IPIPE data */
> +	__u8 offset_ipipe_en;
> +	/* Enable or Disable Gain adjustment for H3A data */
> +	__u8 offset_h3a_en;
> +};
> +
> +struct isif_cul {
> +	/* Horizontal Cull pattern for odd lines */
> +	__u8 hcpat_odd;
> +	/* Horizontal Cull pattern for even lines */
> +	__u8 hcpat_even;
> +	/* Vertical Cull pattern */
> +	__u8 vcpat;
> +	/* Enable or disable lpf. Apply when cull is enabled */
> +	__u8 en_lpf;
> +};
> +
> +struct isif_compress {
> +#define ISIF_ALAW		0
> +#define ISIF_DPCM		1
> +#define ISIF_NO_COMPRESSION	2
> +	/* Compression Algorithm used */
> +	__u8 alg;
> +	/* Choose Predictor1 for DPCM compression */
> +#define ISIF_DPCM_PRED1		0
> +	/* Choose Predictor2 for DPCM compression */
> +#define ISIF_DPCM_PRED2		1
> +	/* Predictor for DPCM compression */
> +	__u8 pred;
> +};
> +
> +/* all the stuff in this struct will be provided by userland */
> +struct isif_config_params_raw {
> +	/* Linearization parameters for image sensor data input */
> +	struct isif_linearize linearize;
> +	/* Data formatter or CSC */
> +	struct isif_df_csc df_csc;
> +	/* Defect Pixel Correction (DFC) confguration */
> +	struct isif_dfc dfc;
> +	/* Black/Digital Clamp configuration */
> +	struct isif_black_clamp bclamp;
> +	/* Gain, offset adjustments */
> +	struct isif_gain_offsets_adj gain_offset;
> +	/* Culling */
> +	struct isif_cul culling;
> +	/* A-Law and DPCM compression options */
> +	struct isif_compress compress;
> +	/* horizontal offset for Gain/LSC/DFC */
> +	__u16 horz_offset;
> +	/* vertical offset for Gain/LSC/DFC */
> +	__u16 vert_offset;
> +	/* color pattern for field 0 */
> +	struct isif_col_pat col_pat_field0;
> +	/* color pattern for field 1 */
> +	struct isif_col_pat col_pat_field1;
> +	/* No Shift */
> +#define ISIF_NO_SHIFT		0
> +	/* 1 bit Shift */
> +#define	ISIF_1BIT_SHIFT		1
> +	/* 2 bit Shift */
> +#define	ISIF_2BIT_SHIFT		2
> +	/* 3 bit Shift */
> +#define	ISIF_3BIT_SHIFT		3
> +	/* 4 bit Shift */
> +#define	ISIF_4BIT_SHIFT		4
> +	/* 5 bit Shift */
> +#define ISIF_5BIT_SHIFT		5
> +	/* 6 bit Shift */
> +#define ISIF_6BIT_SHIFT		6

The comment before each shift define seems rather pointless.

> +	/* Data shift applied before storing to SDRAM */
> +	__u8 data_shift;
> +	/* enable input test pattern generation */
> +	__u8 test_pat_gen;
> +};
> +
> +#ifdef __KERNEL__

Hmm. Is this header supposed to be public or private to the kernel?

> +struct isif_ycbcr_config {
> +	/* isif pixel format */
> +	enum ccdc_pixfmt pix_fmt;
> +	/* isif frame format */
> +	enum ccdc_frmfmt frm_fmt;
> +	/* ISIF crop window */
> +	struct v4l2_rect win;
> +	/* field polarity */
> +	enum vpfe_pin_pol fid_pol;
> +	/* interface VD polarity */
> +	enum vpfe_pin_pol vd_pol;
> +	/* interface HD polarity */
> +	enum vpfe_pin_pol hd_pol;
> +	/* isif pix order. Only used for ycbcr capture */
> +	enum ccdc_pixorder pix_order;
> +	/* isif buffer type. Only used for ycbcr capture */
> +	enum ccdc_buftype buf_type;
> +};
> +
> +/* MSB of image data connected to sensor port */
> +enum isif_data_msb {
> +	/* MSB b15 */
> +	ISIF_BIT_MSB_15,
> +	/* MSB b14 */
> +	ISIF_BIT_MSB_14,
> +	/* MSB b13 */
> +	ISIF_BIT_MSB_13,
> +	/* MSB b12 */
> +	ISIF_BIT_MSB_12,
> +	/* MSB b11 */
> +	ISIF_BIT_MSB_11,
> +	/* MSB b10 */
> +	ISIF_BIT_MSB_10,
> +	/* MSB b9 */
> +	ISIF_BIT_MSB_9,
> +	/* MSB b8 */
> +	ISIF_BIT_MSB_8,
> +	/* MSB b7 */
> +	ISIF_BIT_MSB_7

The comment before each entry seems rather pointless.

> +};
> +
> +enum isif_cfa_pattern {
> +	ISIF_CFA_PAT_MOSAIC,
> +	ISIF_CFA_PAT_STRIPE
> +};
> +
> +struct isif_params_raw {
> +	/* isif pixel format */
> +	enum ccdc_pixfmt pix_fmt;
> +	/* isif frame format */
> +	enum ccdc_frmfmt frm_fmt;
> +	/* video window */
> +	struct v4l2_rect win;
> +	/* field polarity */
> +	enum vpfe_pin_pol fid_pol;
> +	/* interface VD polarity */
> +	enum vpfe_pin_pol vd_pol;
> +	/* interface HD polarity */
> +	enum vpfe_pin_pol hd_pol;
> +	/* buffer type. Applicable for interlaced mode */
> +	enum ccdc_buftype buf_type;
> +	/* Gain values */
> +	struct isif_gain gain;
> +	/* cfa pattern */
> +	enum isif_cfa_pattern cfa_pat;
> +	/* Data MSB position */
> +	enum isif_data_msb data_msb;
> +	/* Enable horizontal flip */
> +	unsigned char horz_flip_en;
> +	/* Enable image invert vertically */
> +	unsigned char image_invert_en;
> +
> +	/* all the userland defined stuff*/
> +	struct isif_config_params_raw config_params;
> +};
> +
> +enum isif_data_pack {
> +	ISIF_PACK_16BIT,
> +	ISIF_PACK_12BIT,
> +	ISIF_PACK_8BIT
> +};
> +
> +#define ISIF_WIN_NTSC				{0, 0, 720, 480}
> +#define ISIF_WIN_VGA				{0, 0, 640, 480}

Please add empty line.

> +#endif
> +#endif
> 

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source
  2009-12-10 17:00   ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source m-karicheri2
  2009-12-10 17:00     ` [PATCH - v1 1/6] V4L - vpfe-capture : DM365 vpss enhancements m-karicheri2
@ 2009-12-15  7:57     ` Hans Verkuil
  1 sibling, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2009-12-15  7:57 UTC (permalink / raw)
  To: m-karicheri2
  Cc: linux-media, khilman, nsekhar, hvaibhav, davinci-linux-open-source

On Thursday 10 December 2009 18:00:26 m-karicheri2@ti.com wrote:
> From: Muralidharan Karicheri <m-karicheri2@ti.com>
> 
> This is the source file for ISIF driver for DM365. This has comments incorporated from
> initial version.
> 
> ISIF driver is equivalent to CCDC driver on DM355 and DM644x. This driver is tested for
> YUV capture from TVP514x driver. This patch contains the header files required for
> this driver. The name of the file is changed to reflect the name of IP.
> 
> Reviewed-by: Nori, Sekhar <nsekhar@ti.com>
> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
> ---
> Applies to linux-next tree of v4l-dvb 
>  drivers/media/video/davinci/isif.c | 1498 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 1498 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/media/video/davinci/isif.c
> 
> diff --git a/drivers/media/video/davinci/isif.c b/drivers/media/video/davinci/isif.c
> new file mode 100644
> index 0000000..916afab
> --- /dev/null
> +++ b/drivers/media/video/davinci/isif.c
> @@ -0,0 +1,1498 @@
> +/*
> + * Copyright (C) 2008-2009 Texas Instruments Inc
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * This is the isif hardware module.
> + * TODO: 1) Raw bayer parameter settings and bayer capture
> + *	 2) Add support for control ioctl
> + */
> +#include <linux/delay.h>
> +#include <linux/platform_device.h>
> +#include <linux/uaccess.h>
> +#include <linux/io.h>
> +#include <linux/videodev2.h>
> +#include <linux/clk.h>
> +
> +#include <mach/mux.h>
> +
> +#include <media/davinci/isif.h>
> +#include <media/davinci/vpss.h>
> +
> +#include "isif_regs.h"
> +#include "ccdc_hw_device.h"
> +
> +/* Defauts for module configuation paramaters */

Typos: 'Defaults', 'configuration' and 'parameters'.

BTW: Please comment somewhere what 'ISIF' stands for!

> +static struct isif_config_params_raw isif_config_defaults = {
> +	.linearize = {
> +		.en = 0,
> +		.corr_shft = ISIF_NO_SHIFT,
> +		.scale_fact = {1, 0},
> +	},
> +	.df_csc = {
> +		.df_or_csc = 0,
> +		.csc = {
> +			.en = 0,
> +		},
> +	},
> +	.dfc = {
> +		.en = 0,
> +	},
> +	.bclamp = {
> +		.en = 0,
> +	},
> +	.gain_offset = {
> +		.gain = {
> +			.r_ye = {1, 0},
> +			.gr_cy = {1, 0},
> +			.gb_g = {1, 0},
> +			.b_mg = {1, 0},
> +		},
> +	},
> +	.culling = {
> +		.hcpat_odd = 0xff,
> +		.hcpat_even = 0xff,
> +		.vcpat = 0xff,
> +	},
> +	.compress = {
> +		.alg = ISIF_ALAW,
> +	},
> +};
> +
> +/* ISIF operation configuration */
> +static struct isif_oper_config {
> +	struct device *dev;
> +	enum vpfe_hw_if_type if_type;
> +	struct isif_ycbcr_config ycbcr;
> +	struct isif_params_raw bayer;
> +	enum isif_data_pack data_pack;
> +	/* Master clock */
> +	struct clk *mclk;
> +	/* ISIF base address */
> +	void __iomem *base_addr;
> +	/* ISIF Linear Table 0 */
> +	void __iomem *linear_tbl0_addr;
> +	/* ISIF Linear Table 1 */
> +	void __iomem *linear_tbl1_addr;
> +} isif_cfg = {
> +	.ycbcr = {
> +		.pix_fmt = CCDC_PIXFMT_YCBCR_8BIT,
> +		.frm_fmt = CCDC_FRMFMT_INTERLACED,
> +		.win = ISIF_WIN_NTSC,
> +		.fid_pol = VPFE_PINPOL_POSITIVE,
> +		.vd_pol = VPFE_PINPOL_POSITIVE,
> +		.hd_pol = VPFE_PINPOL_POSITIVE,
> +		.pix_order = CCDC_PIXORDER_CBYCRY,
> +		.buf_type = CCDC_BUFTYPE_FLD_INTERLEAVED,
> +	},
> +	.bayer = {
> +		.pix_fmt = CCDC_PIXFMT_RAW,
> +		.frm_fmt = CCDC_FRMFMT_PROGRESSIVE,
> +		.win = ISIF_WIN_VGA,
> +		.fid_pol = VPFE_PINPOL_POSITIVE,
> +		.vd_pol = VPFE_PINPOL_POSITIVE,
> +		.hd_pol = VPFE_PINPOL_POSITIVE,
> +		.gain = {
> +			.r_ye = {1, 0},
> +			.gr_cy = {1, 0},
> +			.gb_g = {1, 0},
> +			.b_mg = {1, 0},
> +		},
> +		.cfa_pat = ISIF_CFA_PAT_MOSAIC,
> +		.data_msb = ISIF_BIT_MSB_11,
> +		.config_params = {
> +			.data_shift = ISIF_NO_SHIFT,
> +			.col_pat_field0 = {
> +				.olop = ISIF_GREEN_BLUE,
> +				.olep = ISIF_BLUE,
> +				.elop = ISIF_RED,
> +				.elep = ISIF_GREEN_RED,
> +			},
> +			.col_pat_field1 = {
> +				.olop = ISIF_GREEN_BLUE,
> +				.olep = ISIF_BLUE,
> +				.elop = ISIF_RED,
> +				.elep = ISIF_GREEN_RED,
> +			},
> +			.test_pat_gen = 0,
> +		},
> +	},
> +	.data_pack = ISIF_DATA_PACK8,
> +};
> +
> +/* Raw Bayer formats */
> +static u32 isif_raw_bayer_pix_formats[] =
> +		{V4L2_PIX_FMT_SBGGR8, V4L2_PIX_FMT_SBGGR16};
> +
> +/* Raw YUV formats */
> +static u32 isif_raw_yuv_pix_formats[] =
> +		{V4L2_PIX_FMT_UYVY, V4L2_PIX_FMT_YUYV};

Can these two arrays be const? Just wondering.

> +
> +/* register access routines */
> +static inline u32 regr(u32 offset)
> +{
> +	return __raw_readl(isif_cfg.base_addr + offset);
> +}
> +
> +static inline void regw(u32 val, u32 offset)
> +{
> +	__raw_writel(val, isif_cfg.base_addr + offset);
> +}
> +
> +/* reg_modify() - read, modify and write register */
> +static inline u32 reg_modify(u32 mask, u32 val, u32 offset)
> +{
> +	u32 new_val = (regr(offset) & ~mask) | (val & mask);
> +
> +	regw(new_val, offset);
> +	return new_val;
> +}
> +
> +static inline void regw_lin_tbl(u32 val, u32 offset, int i)
> +{
> +	if (!i)
> +		__raw_writel(val, isif_cfg.linear_tbl0_addr + offset);
> +	else
> +		__raw_writel(val, isif_cfg.linear_tbl1_addr + offset);
> +}
> +
> +static void isif_disable_all_modules(void)
> +{
> +	/* disable BC */
> +	regw(0, CLAMPCFG);
> +	/* disable vdfc */
> +	regw(0, DFCCTL);
> +	/* disable CSC */
> +	regw(0, CSCCTL);
> +	/* disable linearization */
> +	regw(0, LINCFG0);
> +	/* disable other modules here as they are supported */
> +}
> +
> +static void isif_enable(int en)
> +{
> +	if (!en) {
> +		/* Before disable isif, disable all ISIF modules */
> +		isif_disable_all_modules();
> +		/*
> +		 * wait for next VD. Assume lowest scan rate is 12 Hz. So
> +		 * 100 msec delay is good enough
> +		 */
> +		msleep(100);
> +	}
> +	reg_modify(ISIF_SYNCEN_VDHDEN_MASK, en, SYNCEN);
> +}
> +
> +static void isif_enable_output_to_sdram(int en)
> +{
> +	reg_modify(ISIF_SYNCEN_WEN_MASK, en << ISIF_SYNCEN_WEN_SHIFT, SYNCEN);
> +}
> +
> +static void isif_config_culling(struct isif_cul *cul)
> +{
> +	u32 val;
> +
> +	/* Horizontal pattern */
> +	val = (cul->hcpat_even << CULL_PAT_EVEN_LINE_SHIFT) | cul->hcpat_odd;
> +	regw(val, CULH);
> +
> +	/* vertical pattern */
> +	regw(cul->vcpat, CULV);
> +
> +	/* LPF */
> +	reg_modify(ISIF_LPF_MASK << ISIF_LPF_SHIFT,
> +		  cul->en_lpf << ISIF_LPF_SHIFT, MODESET);
> +}
> +
> +static void isif_config_gain_offset(void)
> +{
> +	struct isif_gain_offsets_adj *gain_off_p =
> +		&isif_cfg.bayer.config_params.gain_offset;
> +	u32 val;
> +
> +	val = (!!gain_off_p->gain_sdram_en << GAIN_SDRAM_EN_SHIFT) |
> +		(!!gain_off_p->gain_ipipe_en << GAIN_IPIPE_EN_SHIFT) |
> +		(!!gain_off_p->gain_h3a_en << GAIN_H3A_EN_SHIFT) |
> +		(!!gain_off_p->offset_sdram_en << OFST_SDRAM_EN_SHIFT) |
> +		(!!gain_off_p->offset_ipipe_en << OFST_IPIPE_EN_SHIFT) |
> +		(!!gain_off_p->offset_h3a_en << OFST_H3A_EN_SHIFT);
> +
> +	reg_modify(GAIN_OFFSET_EN_MASK, val, CGAMMAWD);
> +
> +	val = ((gain_off_p->gain.r_ye.integer & GAIN_INTEGER_MASK) <<
> +		GAIN_INTEGER_SHIFT) |
> +		(gain_off_p->gain.r_ye.decimal & GAIN_DECIMAL_MASK);
> +	regw(val, CRGAIN);
> +
> +	val = ((gain_off_p->gain.gr_cy.integer & GAIN_INTEGER_MASK) <<
> +		GAIN_INTEGER_SHIFT) |
> +		(gain_off_p->gain.gr_cy.decimal & GAIN_DECIMAL_MASK);
> +	regw(val, CGRGAIN);
> +
> +	val = ((gain_off_p->gain.gb_g.integer & GAIN_INTEGER_MASK) <<
> +		GAIN_INTEGER_SHIFT) |
> +		(gain_off_p->gain.gb_g.decimal & GAIN_DECIMAL_MASK);
> +	regw(val, CGBGAIN);
> +
> +	val = ((gain_off_p->gain.b_mg.integer & GAIN_INTEGER_MASK) <<
> +		GAIN_INTEGER_SHIFT) |
> +		(gain_off_p->gain.b_mg.decimal & GAIN_DECIMAL_MASK);
> +	regw(val, CBGAIN);
> +
> +	regw((gain_off_p->offset & OFFSET_MASK), COFSTA);
> +}
> +
> +static void isif_restore_defaults(void)
> +{
> +	enum vpss_ccdc_source_sel source = VPSS_CCDCIN;
> +
> +	dev_dbg(isif_cfg.dev, "\nstarting isif_restore_defaults...");
> +	memcpy(&isif_cfg.bayer.config_params, &isif_config_defaults,
> +		sizeof(struct isif_config_params_raw));
> +	/* Enable clock to ISIF, IPIPEIF and BL */
> +	vpss_enable_clock(VPSS_CCDC_CLOCK, 1);
> +	vpss_enable_clock(VPSS_IPIPEIF_CLOCK, 1);
> +	vpss_enable_clock(VPSS_BL_CLOCK, 1);
> +	/* Set default offset and gain */
> +	isif_config_gain_offset();
> +	vpss_select_ccdc_source(source);
> +	dev_dbg(isif_cfg.dev, "\nEnd of isif_restore_defaults...");
> +}
> +
> +static int isif_open(struct device *device)
> +{
> +	isif_restore_defaults();
> +	return 0;
> +}
> +
> +/* This function will configure the window size to be capture in ISIF reg */
> +static void isif_setwin(struct v4l2_rect *image_win,
> +			enum ccdc_frmfmt frm_fmt, int ppc)
> +{
> +	int horz_start, horz_nr_pixels;
> +	int vert_start, vert_nr_lines;
> +	int mid_img = 0;
> +
> +	dev_dbg(isif_cfg.dev, "\nStarting isif_setwin...");
> +	/*
> +	 * ppc - per pixel count. indicates how many pixels per cell
> +	 * output to SDRAM. example, for ycbcr, it is one y and one c, so 2.
> +	 * raw capture this is 1
> +	 */
> +	horz_start = image_win->left << (ppc - 1);
> +	horz_nr_pixels = ((image_win->width) << (ppc - 1)) - 1;
> +
> +	/* Writing the horizontal info into the registers */
> +	regw(horz_start & START_PX_HOR_MASK, SPH);
> +	regw(horz_nr_pixels & NUM_PX_HOR_MASK, LNH);
> +	vert_start = image_win->top;
> +
> +	if (frm_fmt == CCDC_FRMFMT_INTERLACED) {
> +		vert_nr_lines = (image_win->height >> 1) - 1;
> +		vert_start >>= 1;
> +		/* To account for VD since line 0 doesn't have any data */
> +		vert_start += 1;
> +	} else {
> +		/* To account for VD since line 0 doesn't have any data */
> +		vert_start += 1;
> +		vert_nr_lines = image_win->height - 1;
> +		/* configure VDINT0 and VDINT1 */
> +		mid_img = vert_start + (image_win->height / 2);
> +		regw(mid_img, VDINT1);
> +	}
> +
> +	regw(0, VDINT0);
> +	regw(vert_start & START_VER_ONE_MASK, SLV0);
> +	regw(vert_start & START_VER_TWO_MASK, SLV1);
> +	regw(vert_nr_lines & NUM_LINES_VER, LNV);
> +}
> +
> +static void isif_config_bclamp(struct isif_black_clamp *bc)
> +{
> +	u32 val;
> +
> +	/*
> +	 * DC Offset is always added to image data irrespective of bc enable
> +	 * status
> +	 */
> +	val = bc->dc_offset & ISIF_BC_DCOFFSET_MASK;
> +	regw(val, CLDCOFST);
> +
> +	if (bc->en) {
> +		val = (bc->bc_mode_color & ISIF_BC_MODE_COLOR_MASK) <<
> +			ISIF_BC_MODE_COLOR_SHIFT;
> +
> +		/* Enable BC and horizontal clamp caculation paramaters */
> +		val = val | 1 | ((bc->horz.mode & ISIF_HORZ_BC_MODE_MASK) <<
> +			ISIF_HORZ_BC_MODE_SHIFT);
> +
> +		regw(val, CLAMPCFG);
> +
> +		if (bc->horz.mode != ISIF_HORZ_BC_DISABLE) {
> +			/*
> +			 * Window count for calculation
> +			 * Base window selection
> +			 * pixel limit
> +			 * Horizontal size of window
> +			 * vertical size of the window
> +			 * Horizontal start position of the window
> +			 * Vertical start position of the window
> +			 */
> +			val = (bc->horz.win_count_calc &
> +				ISIF_HORZ_BC_WIN_COUNT_MASK) |
> +				((!!bc->horz.base_win_sel_calc) <<
> +				ISIF_HORZ_BC_WIN_SEL_SHIFT) |
> +				((!!bc->horz.clamp_pix_limit) <<
> +				ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
> +				((bc->horz.win_h_sz_calc &
> +				ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
> +				ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
> +				((bc->horz.win_v_sz_calc &
> +				ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
> +				ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
> +
> +			regw(val, CLHWIN0);
> +
> +			regw(bc->horz.win_start_h_calc &
> +			     ISIF_HORZ_BC_WIN_START_H_MASK, CLHWIN1);
> +
> +			regw(bc->horz.win_start_v_calc &
> +			     ISIF_HORZ_BC_WIN_START_V_MASK, CLHWIN2);
> +		}
> +
> +		/* vertical clamp caculation paramaters */
> +
> +		/* OB H Valid */
> +		val = (bc->vert.ob_h_sz_calc & ISIF_VERT_BC_OB_H_SZ_MASK);
> +
> +		/* Reset clamp value sel for previous line */
> +		val |= ((bc->vert.reset_val_sel &
> +			ISIF_VERT_BC_RST_VAL_SEL_MASK) <<
> +			ISIF_VERT_BC_RST_VAL_SEL_SHIFT) |
> +			(bc->vert.line_ave_coef <<
> +			ISIF_VERT_BC_LINE_AVE_COEF_SHIFT);
> +		regw(val, CLVWIN0);
> +
> +		/* Configured reset value */
> +		if (bc->vert.reset_val_sel ==
> +		    ISIF_VERT_BC_USE_CONFIG_CLAMP_VAL) {
> +			regw(bc->vert.reset_clamp_val &
> +			       ISIF_VERT_BC_RST_VAL_MASK, CLVRV);
> +		}
> +
> +		/* Optical Black horizontal start position */
> +		regw(bc->vert.ob_start_h & ISIF_VERT_BC_OB_START_HORZ_MASK,
> +		     CLVWIN1);
> +
> +		/* Optical Black vertical start position */
> +		regw(bc->vert.ob_start_v & ISIF_VERT_BC_OB_START_VERT_MASK,
> +		     CLVWIN2);
> +
> +		regw(bc->vert.ob_v_sz_calc & ISIF_VERT_BC_OB_VERT_SZ_MASK,
> +		     CLVWIN3);
> +
> +		/* Vertical start position for BC subtraction */
> +		regw(bc->vert_start_sub & ISIF_BC_VERT_START_SUB_V_MASK, CLSV);
> +	}
> +}
> +
> +static void isif_config_linearization(struct isif_linearize *linearize)
> +{
> +	u32 val, i;
> +
> +	if (!linearize->en) {
> +		regw(0, LINCFG0);
> +		return;
> +	}
> +
> +	/* shift value for correction & enable linearization (set lsb) */
> +	val = (linearize->corr_shft & ISIF_LIN_CORRSFT_MASK) <<
> +	       ISIF_LIN_CORRSFT_SHIFT | 1;
> +	regw(val, LINCFG0);
> +
> +	/* Scale factor */
> +	val = ((!!linearize->scale_fact.integer) <<
> +	      ISIF_LIN_SCALE_FACT_INTEG_SHIFT) |
> +	      (linearize->scale_fact.decimal &
> +	       ISIF_LIN_SCALE_FACT_DECIMAL_MASK);
> +	regw(val, LINCFG1);
> +
> +	for (i = 0; i < ISIF_LINEAR_TAB_SIZE; i++) {
> +		val = linearize->table[i] & ISIF_LIN_ENTRY_MASK;
> +		if (i % 2)
> +			regw_lin_tbl(val, ((i >> 1) << 2), 1);
> +		else
> +			regw_lin_tbl(val, ((i >> 1) << 2), 0);
> +	}
> +}
> +
> +static int isif_config_dfc(struct isif_dfc *vdfc)
> +{
> +	/* initialize retries to loop for max ~ 250 usec */
> +	u32 val, count, retries = loops_per_jiffy / (4000/HZ);
> +	int i;
> +
> +	if (!vdfc->en)
> +		return 0;
> +
> +	/* Correction mode */
> +	val = ((vdfc->corr_mode & ISIF_VDFC_CORR_MOD_MASK) <<
> +		ISIF_VDFC_CORR_MOD_SHIFT);
> +
> +	/* Correct whole line or partial */
> +	if (vdfc->corr_whole_line)
> +		val |= 1 << ISIF_VDFC_CORR_WHOLE_LN_SHIFT;
> +
> +	/* level shift value */
> +	val |= (vdfc->def_level_shift & ISIF_VDFC_LEVEL_SHFT_MASK) <<
> +		ISIF_VDFC_LEVEL_SHFT_SHIFT;
> +
> +	regw(val, DFCCTL);
> +
> +	/* Defect saturation level */
> +	val = vdfc->def_sat_level & ISIF_VDFC_SAT_LEVEL_MASK;
> +	regw(val, VDFSATLV);
> +
> +	regw(vdfc->table[0].pos_vert & ISIF_VDFC_POS_MASK, DFCMEM0);
> +	regw(vdfc->table[0].pos_horz & ISIF_VDFC_POS_MASK, DFCMEM1);
> +	if (vdfc->corr_mode == ISIF_VDFC_NORMAL ||
> +	    vdfc->corr_mode == ISIF_VDFC_HORZ_INTERPOL_IF_SAT) {
> +		regw(vdfc->table[0].level_at_pos, DFCMEM2);
> +		regw(vdfc->table[0].level_up_pixels, DFCMEM3);
> +		regw(vdfc->table[0].level_low_pixels, DFCMEM4);
> +	}
> +
> +	/* set DFCMARST and set DFCMWR */
> +	val = regr(DFCMEMCTL) | (1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT) | 1;
> +	regw(val, DFCMEMCTL);
> +
> +	count = retries;
> +	while (count && (regr(DFCMEMCTL) & 0x1))
> +		count--;
> +
> +	if (!count) {
> +		dev_dbg(isif_cfg.dev, "defect table write timeout !!!\n");
> +		return -1;
> +	}
> +
> +	for (i = 1; i < vdfc->num_vdefects; i++) {
> +		regw(vdfc->table[i].pos_vert & ISIF_VDFC_POS_MASK,
> +			   DFCMEM0);
> +		regw(vdfc->table[i].pos_horz & ISIF_VDFC_POS_MASK,
> +			   DFCMEM1);
> +		if (vdfc->corr_mode == ISIF_VDFC_NORMAL ||
> +		    vdfc->corr_mode == ISIF_VDFC_HORZ_INTERPOL_IF_SAT) {
> +			regw(vdfc->table[i].level_at_pos, DFCMEM2);
> +			regw(vdfc->table[i].level_up_pixels, DFCMEM3);
> +			regw(vdfc->table[i].level_low_pixels, DFCMEM4);
> +		}
> +		val = regr(DFCMEMCTL);
> +		/* clear DFCMARST and set DFCMWR */
> +		val &= ~BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT);
> +		val |= 1;
> +		regw(val, DFCMEMCTL);
> +
> +		count = retries;
> +		while (count && (regr(DFCMEMCTL) & 0x1))
> +			count--;
> +
> +		if (!count) {
> +			dev_err(isif_cfg.dev,
> +				"defect table write timeout !!!\n");
> +			return -1;
> +		}
> +	}
> +	if (vdfc->num_vdefects < ISIF_VDFC_TABLE_SIZE) {
> +		/* Extra cycle needed */
> +		regw(0, DFCMEM0);
> +		regw(0x1FFF, DFCMEM1);
> +		regw(1, DFCMEMCTL);
> +	}
> +
> +	/* enable VDFC */
> +	reg_modify((1 << ISIF_VDFC_EN_SHIFT), (1 << ISIF_VDFC_EN_SHIFT),
> +		   DFCCTL);
> +	return 0;
> +}
> +
> +static void isif_config_csc(struct isif_df_csc *df_csc)
> +{
> +	u32 val1 = 0, val2 = 0, i;
> +
> +	if (!df_csc->csc.en) {
> +		regw(0, CSCCTL);
> +		return;
> +	}
> +	for (i = 0; i < ISIF_CSC_NUM_COEFF; i++) {
> +		if ((i % 2) == 0) {
> +			/* CSCM - LSB */
> +			val1 = ((df_csc->csc.coeff[i].integer &
> +				ISIF_CSC_COEF_INTEG_MASK) <<
> +				ISIF_CSC_COEF_INTEG_SHIFT) |
> +				((df_csc->csc.coeff[i].decimal &
> +				ISIF_CSC_COEF_DECIMAL_MASK));
> +		} else {
> +
> +			/* CSCM - MSB */
> +			val2 = ((df_csc->csc.coeff[i].integer &
> +				ISIF_CSC_COEF_INTEG_MASK) <<
> +				ISIF_CSC_COEF_INTEG_SHIFT) |
> +				((df_csc->csc.coeff[i].decimal &
> +				ISIF_CSC_COEF_DECIMAL_MASK));
> +			val2 <<= ISIF_CSCM_MSB_SHIFT;
> +			val2 |= val1;
> +			regw(val2, (CSCM0 + ((i - 1) << 1)));
> +		}
> +	}
> +
> +	/* program the active area */
> +	regw(df_csc->start_pix & ISIF_DF_CSC_SPH_MASK, FMTSPH);
> +	/*
> +	 * one extra pixel as required for CSC. Actually number of
> +	 * pixel - 1 should be configured in this register. So we
> +	 * need to subtract 1 before writing to FMTSPH, but we will
> +	 * not do this since csc requires one extra pixel
> +	 */
> +	regw((df_csc->num_pixels) & ISIF_DF_CSC_SPH_MASK, FMTLNH);
> +	regw(df_csc->start_line & ISIF_DF_CSC_SPH_MASK, FMTSLV);
> +	/*
> +	 * one extra line as required for CSC. See reason documented for
> +	 * num_pixels
> +	 */
> +	regw((df_csc->num_lines) & ISIF_DF_CSC_SPH_MASK, FMTLNV);
> +
> +	/* Enable CSC */
> +	regw(1, CSCCTL);
> +}
> +
> +static int isif_config_raw(void)
> +{
> +	struct isif_params_raw *params = &isif_cfg.bayer;
> +	struct isif_config_params_raw *module_params =
> +		&isif_cfg.bayer.config_params;
> +	struct vpss_pg_frame_size frame_size;
> +	struct vpss_sync_pol sync;
> +	u32 val;
> +
> +	dev_dbg(isif_cfg.dev, "\nStarting isif_config_raw..\n");
> +
> +	/*
> +	 * Configure CCDCFG register:-
> +	 * Set CCD Not to swap input since input is RAW data
> +	 * Set FID detection function to Latch at V-Sync
> +	 * Set WENLOG - isif valid area
> +	 * Set TRGSEL
> +	 * Set EXTRG
> +	 * Packed to 8 or 16 bits
> +	 */
> +
> +	val = ISIF_YCINSWP_RAW | ISIF_CCDCFG_FIDMD_LATCH_VSYNC |
> +		ISIF_CCDCFG_WENLOG_AND | ISIF_CCDCFG_TRGSEL_WEN |
> +		ISIF_CCDCFG_EXTRG_DISABLE | (isif_cfg.data_pack &
> +		ISIF_DATA_PACK_MASK);
> +
> +	dev_dbg(isif_cfg.dev, "Writing 0x%x to ...CCDCFG \n", val);
> +	regw(val, CCDCFG);
> +
> +	/*
> +	 * Configure the vertical sync polarity(MODESET.VDPOL)
> +	 * Configure the horizontal sync polarity (MODESET.HDPOL)
> +	 * Configure frame id polarity (MODESET.FLDPOL)
> +	 * Configure data polarity
> +	 * Configure External WEN Selection
> +	 * Configure frame format(progressive or interlace)
> +	 * Configure pixel format (Input mode)
> +	 * Configure the data shift
> +	 */
> +
> +	val = ISIF_VDHDOUT_INPUT |
> +		((params->vd_pol & ISIF_VD_POL_MASK) << ISIF_VD_POL_SHIFT) |
> +		((params->hd_pol & ISIF_HD_POL_MASK) << ISIF_HD_POL_SHIFT) |
> +		((params->fid_pol & ISIF_FID_POL_MASK) << ISIF_FID_POL_SHIFT) |
> +		((ISIF_DATAPOL_NORMAL & ISIF_DATAPOL_MASK) <<
> +		ISIF_DATAPOL_SHIFT) |
> +		((ISIF_EXWEN_DISABLE & ISIF_EXWEN_MASK) << ISIF_EXWEN_SHIFT) |
> +		((params->frm_fmt & ISIF_FRM_FMT_MASK) << ISIF_FRM_FMT_SHIFT) |
> +		((params->pix_fmt & ISIF_INPUT_MASK) << ISIF_INPUT_SHIFT) |
> +		((params->config_params.data_shift & ISIF_DATASFT_MASK) <<
> +		ISIF_DATASFT_SHIFT);
> +
> +	regw(val, MODESET);
> +	dev_dbg(isif_cfg.dev, "Writing 0x%x to MODESET...\n", val);
> +
> +	/*
> +	 * Configure GAMMAWD register
> +	 * CFA pattern setting
> +	 */
> +	val = (params->cfa_pat & ISIF_GAMMAWD_CFA_MASK) <<
> +		ISIF_GAMMAWD_CFA_SHIFT;
> +
> +	/* Gamma msb */
> +	if (module_params->compress.alg == ISIF_ALAW)
> +		val |= ISIF_ALAW_ENABLE;
> +
> +	val |= ((params->data_msb & ISIF_ALAW_GAMA_WD_MASK) <<
> +			ISIF_ALAW_GAMA_WD_SHIFT);
> +
> +	regw(val, CGAMMAWD);
> +
> +	/* Configure DPCM compression settings */
> +	if (module_params->compress.alg == ISIF_DPCM) {
> +		val =  BIT(ISIF_DPCM_EN_SHIFT) |
> +		       ((module_params->compress.pred &
> +			ISIF_DPCM_PREDICTOR_MASK) << ISIF_DPCM_PREDICTOR_SHIFT);
> +	}
> +
> +	regw(val, MISC);
> +
> +	/* Configure Gain & Offset */
> +	isif_config_gain_offset();
> +
> +	/* Configure Color pattern */
> +	val = (params->config_params.col_pat_field0.olop) |
> +	(params->config_params.col_pat_field0.olep << 2) |
> +	(params->config_params.col_pat_field0.elop << 4) |
> +	(params->config_params.col_pat_field0.elep << 6) |
> +	(params->config_params.col_pat_field1.olop << 8) |
> +	(params->config_params.col_pat_field1.olep << 10) |
> +	(params->config_params.col_pat_field1.elop << 12) |
> +	(params->config_params.col_pat_field1.elep << 14);
> +	regw(val, CCOLP);
> +	dev_dbg(isif_cfg.dev, "Writing %x to CCOLP ...\n", val);
> +
> +	/* Configure HSIZE register  */
> +	val = (params->horz_flip_en & ISIF_HSIZE_FLIP_MASK) <<
> +		ISIF_HSIZE_FLIP_SHIFT;
> +
> +	/* calculate line offset in 32 bytes based on pack value */
> +	if (isif_cfg.data_pack == ISIF_PACK_8BIT)
> +		val |= (((params->win.width + 31) >> 5) & ISIF_LINEOFST_MASK);
> +	else if (isif_cfg.data_pack == ISIF_PACK_12BIT)
> +		val |= ((((params->win.width +
> +			(params->win.width >> 2)) + 31) >> 5) &
> +			ISIF_LINEOFST_MASK);
> +	else
> +		val |= ((((params->win.width * 2) + 31) >> 5) &
> +			ISIF_LINEOFST_MASK);
> +	regw(val, HSIZE);
> +
> +	/* Configure SDOFST register  */
> +	if (params->frm_fmt == CCDC_FRMFMT_INTERLACED) {
> +		if (params->image_invert_en) {
> +			/* For interlace inverse mode */
> +			regw(0x4B6D, SDOFST);
> +			dev_dbg(isif_cfg.dev, "Writing 0x4B6D to SDOFST...\n");
> +		} else {
> +			/* For interlace non inverse mode */
> +			regw(0x0B6D, SDOFST);
> +			dev_dbg(isif_cfg.dev, "Writing 0x0B6D to SDOFST...\n");
> +		}
> +	} else if (params->frm_fmt == CCDC_FRMFMT_PROGRESSIVE) {
> +		if (params->image_invert_en) {
> +			/* For progessive inverse mode */

Typo: progressive

> +			regw(0x4000, SDOFST);
> +			dev_dbg(isif_cfg.dev, "Writing 0x4000 to SDOFST...\n");
> +		} else {
> +			/* For progessive non inverse mode */
> +			regw(0x0000, SDOFST);
> +			dev_dbg(isif_cfg.dev, "Writing 0x0000 to SDOFST...\n");
> +		}
> +	}
> +
> +	/* Configure video window */
> +	isif_setwin(&params->win, params->frm_fmt, 1);
> +
> +	/* Configure Black Clamp */
> +	isif_config_bclamp(&module_params->bclamp);
> +
> +	/* Configure Vertical Defection Pixel Correction */
> +	if (isif_config_dfc(&module_params->dfc) < 0)
> +		return -EFAULT;
> +
> +	if (!module_params->df_csc.df_or_csc)
> +		/* Configure Color Space Conversion */
> +		isif_config_csc(&module_params->df_csc);
> +
> +	isif_config_linearization(&module_params->linearize);
> +
> +	/* Configure Culling */
> +	isif_config_culling(&module_params->culling);
> +
> +	/* Configure Horizontal and vertical offsets(DFC,LSC,Gain) */
> +	val = module_params->horz_offset & ISIF_DATA_H_OFFSET_MASK;
> +	regw(val, DATAHOFST);
> +
> +	val = module_params->vert_offset & ISIF_DATA_V_OFFSET_MASK;
> +	regw(val, DATAVOFST);
> +
> +	/* Setup test pattern if enabled */
> +	if (params->config_params.test_pat_gen) {
> +		/* Use the HD/VD pol settings from user */
> +		sync.ccdpg_hdpol = params->hd_pol & ISIF_HD_POL_MASK;
> +		sync.ccdpg_vdpol = params->vd_pol & ISIF_VD_POL_MASK;
> +
> +		dm365_vpss_set_sync_pol(sync);
> +
> +		frame_size.hlpfr = isif_cfg.bayer.win.width;
> +		frame_size.pplen = isif_cfg.bayer.win.height;
> +		dm365_vpss_set_pg_frame_size(frame_size);
> +		vpss_select_ccdc_source(VPSS_PGLPBK);
> +	}
> +
> +	dev_dbg(isif_cfg.dev, "\nEnd of isif_config_ycbcr...\n");
> +	return 0;
> +}
> +
> +static int isif_validate_df_csc_params(struct isif_df_csc *df_csc)
> +{
> +	struct isif_color_space_conv *csc;
> +	int i, csc_df_en = 0;
> +	int err = -EINVAL;
> +
> +	if (!df_csc->df_or_csc) {
> +		/* csc configuration */
> +		csc = &df_csc->csc;
> +		if (csc->en) {

You can save one level of indent by doing:

if (!df_csc->df_or_csc && df_csc->csc.en)

> +			csc_df_en = 1;
> +			for (i = 0; i < ISIF_CSC_NUM_COEFF; i++) {
> +				if (csc->coeff[i].integer >
> +					ISIF_CSC_COEF_INTEG_MASK ||
> +				    csc->coeff[i].decimal >
> +					ISIF_CSC_COEF_DECIMAL_MASK) {
> +					dev_dbg(isif_cfg.dev,
> +					       "invalid csc coefficients \n");
> +					return err;
> +				}
> +			}
> +		}
> +	}
> +
> +	if (df_csc->start_pix > ISIF_DF_CSC_SPH_MASK) {
> +		dev_dbg(isif_cfg.dev, "invalid df_csc start pix value \n");
> +		return err;
> +	}
> +	if (df_csc->num_pixels > ISIF_DF_NUMPIX) {
> +		dev_dbg(isif_cfg.dev, "invalid df_csc num pixels value \n");
> +		return err;
> +	}
> +	if (df_csc->start_line > ISIF_DF_CSC_LNH_MASK) {
> +		dev_dbg(isif_cfg.dev, "invalid df_csc start_line value \n");
> +		return err;
> +	}
> +	if (df_csc->num_lines > ISIF_DF_NUMLINES) {
> +		dev_dbg(isif_cfg.dev, "invalid df_csc num_lines value \n");
> +		return err;
> +	}
> +	return 0;
> +}
> +
> +static int isif_validate_dfc_params(struct isif_dfc *dfc)
> +{
> +	int err = -EINVAL;
> +	int i;
> +
> +	if (dfc->en) {

Please invert:

	if (!dfc->en)
		return 0;

The remainder of this function can now be shifted one tab to the left.

> +		if (dfc->corr_whole_line > 1) {
> +			dev_dbg(isif_cfg.dev,
> +				"invalid corr_whole_line value\n");
> +			return err;
> +		}
> +
> +		if (dfc->def_level_shift > 4) {
> +			dev_dbg(isif_cfg.dev,
> +				"invalid def_level_shift value\n");
> +			return err;
> +		}
> +
> +		if (dfc->def_sat_level > 4095) {
> +			dev_dbg(isif_cfg.dev, "invalid def_sat_level value \n");
> +			return err;
> +		}
> +		if ((!dfc->num_vdefects) || (dfc->num_vdefects > 8)) {
> +			dev_dbg(isif_cfg.dev, "invalid num_vdefects value \n");
> +			return err;
> +		}
> +		for (i = 0; i < ISIF_VDFC_TABLE_SIZE; i++) {
> +			if (dfc->table[i].pos_vert > 0x1fff) {
> +				dev_dbg(isif_cfg.dev,
> +					"invalid pos_vert value \n");
> +				return err;
> +			}
> +			if (dfc->table[i].pos_horz > 0x1fff) {
> +				dev_dbg(isif_cfg.dev,
> +					"invalid pos_horz value \n");
> +				return err;
> +			}
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int isif_validate_bclamp_params(struct isif_black_clamp *bclamp)
> +{
> +	int err = -EINVAL;
> +
> +	if (bclamp->dc_offset > 0x1fff) {
> +		dev_dbg(isif_cfg.dev, "invalid bclamp dc_offset value \n");
> +		return err;
> +	}
> +
> +	if (bclamp->en) {

Ditto.

> +		if (bclamp->horz.clamp_pix_limit > 1) {
> +			dev_dbg(isif_cfg.dev,
> +			       "invalid bclamp horz clamp_pix_limit value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->horz.win_count_calc < 1 ||
> +		    bclamp->horz.win_count_calc > 32) {
> +			dev_dbg(isif_cfg.dev,
> +			       "invalid bclamp horz win_count_calc value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->horz.win_start_h_calc > 0x1fff) {
> +			dev_dbg(isif_cfg.dev,
> +			       "invalid bclamp win_start_v_calc value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->horz.win_start_v_calc > 0x1fff) {
> +			dev_dbg(isif_cfg.dev,
> +			       "invalid bclamp win_start_v_calc value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->vert.reset_clamp_val > 0xfff) {
> +			dev_dbg(isif_cfg.dev,
> +			       "invalid bclamp reset_clamp_val value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->vert.ob_v_sz_calc > 0x1fff) {
> +			dev_dbg(isif_cfg.dev,
> +				"invalid bclamp ob_v_sz_calc value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->vert.ob_start_h > 0x1fff) {
> +			dev_dbg(isif_cfg.dev,
> +				"invalid bclamp ob_start_h value \n");
> +			return err;
> +		}
> +
> +		if (bclamp->vert.ob_start_v > 0x1fff) {
> +			dev_dbg(isif_cfg.dev,
> +				"invalid bclamp ob_start_h value \n");
> +			return err;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int isif_validate_gain_ofst_params(struct isif_gain_offsets_adj
> +					  *gain_offset)
> +{
> +	int err = -EINVAL;
> +
> +	if (gain_offset->gain_sdram_en ||
> +	    gain_offset->gain_ipipe_en ||
> +	    gain_offset->gain_h3a_en) {
> +		if ((gain_offset->gain.r_ye.integer > 7) ||
> +		    (gain_offset->gain.r_ye.decimal > 0x1ff)) {
> +			dev_dbg(isif_cfg.dev, "invalid  gain r_ye\n");
> +			return err;
> +		}
> +		if ((gain_offset->gain.gr_cy.integer > 7) ||
> +		    (gain_offset->gain.gr_cy.decimal > 0x1ff)) {
> +			dev_dbg(isif_cfg.dev, "invalid  gain gr_cy\n");
> +			return err;
> +		}
> +		if ((gain_offset->gain.gb_g.integer > 7) ||
> +		    (gain_offset->gain.gb_g.decimal > 0x1ff)) {
> +			dev_dbg(isif_cfg.dev, "invalid  gain gb_g\n");
> +			return err;
> +		}
> +		if ((gain_offset->gain.b_mg.integer > 7) ||
> +		    (gain_offset->gain.b_mg.decimal > 0x1ff)) {
> +			dev_dbg(isif_cfg.dev, "invalid  gain b_mg\n");
> +			return err;
> +		}
> +	}
> +	if (gain_offset->offset_sdram_en ||
> +	    gain_offset->offset_ipipe_en ||
> +	    gain_offset->offset_h3a_en) {
> +		if (gain_offset->offset > 0xfff) {
> +			dev_dbg(isif_cfg.dev, "invalid  gain b_mg\n");
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +validate_isif_config_params_raw(struct isif_config_params_raw *params)
> +{
> +	int err;
> +
> +	err = isif_validate_df_csc_params(&params->df_csc);
> +	if (err)
> +		goto exit;

Why goto? Just return here. Ditto for the next two gotos.

> +	err = isif_validate_dfc_params(&params->dfc);
> +	if (err)
> +		goto exit;
> +	err = isif_validate_bclamp_params(&params->bclamp);
> +	if (err)
> +		goto exit;
> +	err = isif_validate_gain_ofst_params(&params->gain_offset);
> +exit:
> +	return err;
> +}
> +
> +static int isif_set_buftype(enum ccdc_buftype buf_type)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		isif_cfg.bayer.buf_type = buf_type;
> +	else
> +		isif_cfg.ycbcr.buf_type = buf_type;
> +
> +	return 0;
> +
> +}
> +static enum ccdc_buftype isif_get_buftype(void)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		return isif_cfg.bayer.buf_type;
> +
> +	return isif_cfg.ycbcr.buf_type;
> +}
> +
> +static int isif_enum_pix(u32 *pix, int i)
> +{
> +	int ret = -EINVAL;
> +
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
> +		if (i < ARRAY_SIZE(isif_raw_bayer_pix_formats)) {
> +			*pix = isif_raw_bayer_pix_formats[i];
> +			ret = 0;
> +		}
> +	} else {
> +		if (i < ARRAY_SIZE(isif_raw_yuv_pix_formats)) {
> +			*pix = isif_raw_yuv_pix_formats[i];
> +			ret = 0;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int isif_set_pixel_format(unsigned int pixfmt)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
> +		if (pixfmt == V4L2_PIX_FMT_SBGGR8) {
> +			if ((isif_cfg.bayer.config_params.compress.alg !=
> +			     ISIF_ALAW) &&
> +			    (isif_cfg.bayer.config_params.compress.alg !=
> +			     ISIF_DPCM)) {
> +				dev_dbg(isif_cfg.dev,
> +					"Either configure A-Law or DPCM\n");
> +				return -EINVAL;
> +			}
> +			isif_cfg.data_pack = ISIF_PACK_8BIT;
> +		} else if (pixfmt == V4L2_PIX_FMT_SBGGR16) {
> +			isif_cfg.bayer.config_params.compress.alg =
> +					ISIF_NO_COMPRESSION;
> +			isif_cfg.data_pack = ISIF_PACK_16BIT;
> +		} else
> +			return -EINVAL;
> +		isif_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
> +	} else {
> +		if (pixfmt == V4L2_PIX_FMT_YUYV)
> +			isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_YCBYCR;
> +		else if (pixfmt == V4L2_PIX_FMT_UYVY)
> +			isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
> +		else
> +			return -EINVAL;
> +		isif_cfg.data_pack = ISIF_PACK_8BIT;
> +	}
> +	return 0;
> +}
> +
> +static u32 isif_get_pixel_format(void)
> +{
> +	u32 pixfmt;
> +
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		if (isif_cfg.bayer.config_params.compress.alg
> +			== ISIF_ALAW
> +			|| isif_cfg.bayer.config_params.compress.alg
> +			== ISIF_DPCM)
> +				pixfmt = V4L2_PIX_FMT_SBGGR8;
> +		else
> +			pixfmt = V4L2_PIX_FMT_SBGGR16;
> +	else {
> +		if (isif_cfg.ycbcr.pix_order == CCDC_PIXORDER_YCBYCR)
> +			pixfmt = V4L2_PIX_FMT_YUYV;
> +		else
> +			pixfmt = V4L2_PIX_FMT_UYVY;
> +	}
> +	return pixfmt;
> +}
> +
> +static int isif_set_image_window(struct v4l2_rect *win)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
> +		isif_cfg.bayer.win.top = win->top;
> +		isif_cfg.bayer.win.left = win->left;
> +		isif_cfg.bayer.win.width = win->width;
> +		isif_cfg.bayer.win.height = win->height;
> +	} else {
> +		isif_cfg.ycbcr.win.top = win->top;
> +		isif_cfg.ycbcr.win.left = win->left;
> +		isif_cfg.ycbcr.win.width = win->width;
> +		isif_cfg.ycbcr.win.height = win->height;
> +	}
> +	return 0;
> +}
> +
> +static void isif_get_image_window(struct v4l2_rect *win)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		*win = isif_cfg.bayer.win;
> +	else
> +		*win = isif_cfg.ycbcr.win;
> +}
> +
> +static unsigned int isif_get_line_length(void)
> +{
> +	unsigned int len;
> +
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER) {
> +		if (isif_cfg.data_pack == ISIF_PACK_8BIT)
> +			len = ((isif_cfg.bayer.win.width));
> +		else if (isif_cfg.data_pack == ISIF_PACK_12BIT)
> +			len = (((isif_cfg.bayer.win.width * 2) +
> +				 (isif_cfg.bayer.win.width >> 2)));
> +		else
> +			len = (((isif_cfg.bayer.win.width * 2)));
> +	} else
> +		len = (((isif_cfg.ycbcr.win.width * 2)));
> +
> +	return ALIGN(len, 32);
> +}
> +
> +static int isif_set_frame_format(enum ccdc_frmfmt frm_fmt)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		isif_cfg.bayer.frm_fmt = frm_fmt;
> +	else
> +		isif_cfg.ycbcr.frm_fmt = frm_fmt;
> +
> +	return 0;
> +}
> +static enum ccdc_frmfmt isif_get_frame_format(void)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		return isif_cfg.bayer.frm_fmt;
> +	else

'else' is not needed here.

> +		return isif_cfg.ycbcr.frm_fmt;
> +}
> +
> +static int isif_getfid(void)
> +{
> +	return (regr(MODESET) >> 15) & 0x1;
> +}
> +
> +/* misc operations */
> +static void isif_setfbaddr(unsigned long addr)
> +{
> +	regw((addr >> 21) & 0x07ff, CADU);
> +	regw((addr >> 5) & 0x0ffff, CADL);
> +}
> +
> +static int isif_set_hw_if_params(struct vpfe_hw_if_param *params)
> +{
> +	isif_cfg.if_type = params->if_type;
> +
> +	switch (params->if_type) {
> +	case VPFE_BT656:
> +	case VPFE_BT656_10BIT:
> +	case VPFE_YCBCR_SYNC_8:
> +		isif_cfg.ycbcr.pix_fmt = CCDC_PIXFMT_YCBCR_8BIT;
> +		isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
> +		break;
> +	case VPFE_BT1120:
> +	case VPFE_YCBCR_SYNC_16:
> +		isif_cfg.ycbcr.pix_fmt = CCDC_PIXFMT_YCBCR_16BIT;
> +		isif_cfg.ycbcr.pix_order = CCDC_PIXORDER_CBYCRY;
> +		break;
> +	case VPFE_RAW_BAYER:
> +		isif_cfg.bayer.pix_fmt = CCDC_PIXFMT_RAW;
> +		break;
> +	default:
> +		dev_dbg(isif_cfg.dev, "Invalid interface type\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Parameter operations */
> +static int isif_get_params(void __user *params)
> +{
> +	/* only raw module parameters can be set through the IOCTL */
> +	if (isif_cfg.if_type != VPFE_RAW_BAYER)
> +		return -EINVAL;
> +
> +	if (copy_to_user(params,
> +			&isif_cfg.bayer.config_params,
> +			sizeof(isif_cfg.bayer.config_params))) {
> +		dev_dbg(isif_cfg.dev,
> +			"isif_get_params: error in copying isif params\n");
> +		return -EFAULT;
> +	}
> +	return 0;
> +}
> +
> +/* Parameter operations */
> +static int isif_set_params(void __user *params)
> +{
> +	struct isif_config_params_raw *isif_raw_params;
> +	int ret = -EINVAL;
> +
> +	/* only raw module parameters can be set through the IOCTL */
> +	if (isif_cfg.if_type != VPFE_RAW_BAYER)
> +		return ret;
> +
> +	isif_raw_params = kzalloc(sizeof(*isif_raw_params), GFP_KERNEL);
> +
> +	if (NULL == isif_raw_params)
> +		return -ENOMEM;
> +
> +	ret = copy_from_user(isif_raw_params,
> +			     params, sizeof(*isif_raw_params));
> +	if (ret) {
> +		dev_dbg(isif_cfg.dev, "isif_set_params: error in copying isif"
> +			"params, %d\n", ret);
> +		ret = -EFAULT;
> +		goto free_out;
> +	}
> +
> +	if (!validate_isif_config_params_raw(isif_raw_params)) {
> +		memcpy(&isif_cfg.bayer.config_params,
> +			isif_raw_params,
> +			sizeof(*isif_raw_params));
> +		ret = 0;
> +	} else
> +		ret = -EINVAL;
> +free_out:
> +	kfree(isif_raw_params);
> +	return ret;
> +}
> +
> +/* This function will configure ISIF for YCbCr parameters. */
> +static int isif_config_ycbcr(void)
> +{
> +	struct isif_ycbcr_config *params = &isif_cfg.ycbcr;
> +	struct vpss_pg_frame_size frame_size;
> +	u32 modeset = 0, ccdcfg = 0;
> +	struct vpss_sync_pol sync;
> +
> +	dev_dbg(isif_cfg.dev, "\nStarting isif_config_ycbcr...");
> +
> +	/* configure pixel format or input mode */
> +	modeset = modeset | ((params->pix_fmt & ISIF_INPUT_MASK)
> +		<< ISIF_INPUT_SHIFT) |
> +	((params->frm_fmt & ISIF_FRM_FMT_MASK) << ISIF_FRM_FMT_SHIFT) |
> +	(((params->fid_pol & ISIF_FID_POL_MASK) << ISIF_FID_POL_SHIFT))	|
> +	(((params->hd_pol & ISIF_HD_POL_MASK) << ISIF_HD_POL_SHIFT)) |
> +	(((params->vd_pol & ISIF_VD_POL_MASK) << ISIF_VD_POL_SHIFT));
> +
> +	/* pack the data to 8-bit ISIFCFG */
> +	switch (isif_cfg.if_type) {
> +	case VPFE_BT656:
> +		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
> +			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
> +			return -1;
> +		}
> +		modeset |=
> +			((VPFE_PINPOL_NEGATIVE & ISIF_VD_POL_MASK)
> +			<< ISIF_VD_POL_SHIFT);
> +		regw(3, REC656IF);
> +		ccdcfg = ccdcfg | ISIF_DATA_PACK8 | ISIF_YCINSWP_YCBCR;
> +		break;
> +	case VPFE_BT656_10BIT:
> +		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
> +			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
> +			return -1;
> +		}
> +		/* setup BT.656, embedded sync  */
> +		regw(3, REC656IF);
> +		/* enable 10 bit mode in ccdcfg */
> +		ccdcfg = ccdcfg | ISIF_DATA_PACK8 | ISIF_YCINSWP_YCBCR |
> +			ISIF_BW656_ENABLE;
> +		break;
> +	case VPFE_BT1120:
> +		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_16BIT) {
> +			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
> +			return -EINVAL;
> +		}
> +		regw(3, REC656IF);
> +		break;
> +
> +	case VPFE_YCBCR_SYNC_8:
> +		ccdcfg |= ISIF_DATA_PACK8;
> +		ccdcfg |= ISIF_YCINSWP_YCBCR;
> +		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_8BIT) {
> +			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
> +			return -EINVAL;
> +		}
> +		break;
> +	case VPFE_YCBCR_SYNC_16:
> +		if (params->pix_fmt != CCDC_PIXFMT_YCBCR_16BIT) {
> +			dev_dbg(isif_cfg.dev, "Invalid pix_fmt(input mode)\n");
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		/* should never come here */
> +		dev_dbg(isif_cfg.dev, "Invalid interface type\n");
> +		return -EINVAL;
> +	}
> +
> +	regw(modeset, MODESET);
> +
> +	/* Set up pix order */
> +	ccdcfg |= (params->pix_order & ISIF_PIX_ORDER_MASK) <<
> +		ISIF_PIX_ORDER_SHIFT;
> +
> +	regw(ccdcfg, CCDCFG);
> +
> +	/* configure video window */
> +	if ((isif_cfg.if_type == VPFE_BT1120) ||
> +	    (isif_cfg.if_type == VPFE_YCBCR_SYNC_16))
> +		isif_setwin(&params->win, params->frm_fmt, 1);
> +	else
> +		isif_setwin(&params->win, params->frm_fmt, 2);
> +
> +	/*
> +	 * configure the horizontal line offset
> +	 * this is done by rounding up width to a multiple of 16 pixels
> +	 * and multiply by two to account for y:cb:cr 4:2:2 data
> +	 */
> +	regw(((((params->win.width * 2) + 31) & 0xffffffe0) >> 5), HSIZE);
> +
> +	/* configure the memory line offset */
> +	if ((params->frm_fmt == CCDC_FRMFMT_INTERLACED) &&
> +	    (params->buf_type == CCDC_BUFTYPE_FLD_INTERLEAVED))
> +		/* two fields are interleaved in memory */
> +		regw(0x00000249, SDOFST);
> +
> +	/* Setup test pattern if enabled */
> +	if (isif_cfg.bayer.config_params.test_pat_gen) {
> +		sync.ccdpg_hdpol = (params->hd_pol & ISIF_HD_POL_MASK);
> +		sync.ccdpg_vdpol = (params->vd_pol & ISIF_VD_POL_MASK);
> +		dm365_vpss_set_sync_pol(sync);
> +		dm365_vpss_set_pg_frame_size(frame_size);
> +	}
> +
> +	return 0;
> +}
> +
> +static int isif_configure(void)
> +{
> +	if (isif_cfg.if_type == VPFE_RAW_BAYER)
> +		return isif_config_raw();
> +	else

'else' not needed here.

> +		isif_config_ycbcr();
> +	return 0;
> +}
> +
> +static int isif_close(struct device *device)
> +{
> +	/* copy defaults to module params */
> +	memcpy(&isif_cfg.bayer.config_params,
> +	       &isif_config_defaults,
> +	       sizeof(struct isif_config_params_raw));

Can you also do: isif_cfg.bayer.config_params = isif_config_defaults?

> +	return 0;
> +}
> +
> +static struct ccdc_hw_device isif_hw_dev = {
> +	.name = "ISIF",
> +	.owner = THIS_MODULE,
> +	.hw_ops = {
> +		.open = isif_open,
> +		.close = isif_close,
> +		.enable = isif_enable,
> +		.enable_out_to_sdram = isif_enable_output_to_sdram,
> +		.set_hw_if_params = isif_set_hw_if_params,
> +		.set_params = isif_set_params,
> +		.get_params = isif_get_params,
> +		.configure = isif_configure,
> +		.set_buftype = isif_set_buftype,
> +		.get_buftype = isif_get_buftype,
> +		.enum_pix = isif_enum_pix,
> +		.set_pixel_format = isif_set_pixel_format,
> +		.get_pixel_format = isif_get_pixel_format,
> +		.set_frame_format = isif_set_frame_format,
> +		.get_frame_format = isif_get_frame_format,
> +		.set_image_window = isif_set_image_window,
> +		.get_image_window = isif_get_image_window,
> +		.get_line_length = isif_get_line_length,
> +		.setfbaddr = isif_setfbaddr,
> +		.getfid = isif_getfid,
> +	},
> +};
> +
> +static int __init isif_probe(struct platform_device *pdev)
> +{
> +	void (*setup_pinmux)(void);
> +	struct resource	*res;
> +	void *__iomem addr;
> +	int status = 0, i;
> +
> +	/*
> +	 * first try to register with vpfe. If not correct platform, then we
> +	 * don't have to iomap
> +	 */
> +	status = vpfe_register_ccdc_device(&isif_hw_dev);
> +	if (status < 0)
> +		return status;
> +
> +	/* Get and enable Master clock */
> +	isif_cfg.mclk = clk_get(&pdev->dev, "master");
> +	if (NULL == isif_cfg.mclk) {
> +		status = -ENODEV;
> +		goto fail_mclk;
> +	}
> +	if (clk_enable(isif_cfg.mclk)) {
> +		status = -ENODEV;
> +		goto fail_mclk;
> +	}
> +
> +	/* Platform data holds setup_pinmux function ptr */
> +	if (NULL == pdev->dev.platform_data) {
> +		status = -ENODEV;
> +		goto fail_mclk;
> +	}
> +	setup_pinmux = pdev->dev.platform_data;
> +	/*
> +	 * setup Mux configuration for ccdc which may be different for
> +	 * different SoCs using this CCDC
> +	 */
> +	setup_pinmux();
> +
> +	i = 0;
> +	/* Get the ISIF base address, linearization table0 and table1 addr. */
> +	while (i < 3) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		if (!res) {
> +			status = -ENODEV;
> +			goto fail_nobase_res;
> +		}
> +		res = request_mem_region(res->start, resource_size(res),
> +					 res->name);
> +		if (!res) {
> +			status = -EBUSY;
> +			goto fail_nobase_res;
> +		}
> +		addr = ioremap_nocache(res->start, resource_size(res));
> +		if (!addr) {
> +			status = -ENOMEM;
> +			goto fail_base_iomap;
> +		}
> +		switch (i) {
> +		case 0:
> +			/* ISIF base address */
> +			isif_cfg.base_addr = addr;
> +			break;
> +		case 1:
> +			/* ISIF linear tbl0 address */
> +			isif_cfg.linear_tbl0_addr = addr;
> +			break;
> +		default:
> +			/* ISIF linear tbl0 address */
> +			isif_cfg.linear_tbl1_addr = addr;
> +			break;
> +		}
> +		i++;
> +	}
> +	isif_cfg.dev = &pdev->dev;
> +
> +	printk(KERN_NOTICE "%s is registered with vpfe.\n",
> +		isif_hw_dev.name);
> +	return 0;
> +fail_base_iomap:
> +	release_mem_region(res->start, resource_size(res));
> +	i--;
> +fail_nobase_res:
> +	if (isif_cfg.base_addr)
> +		iounmap(isif_cfg.base_addr);
> +	if (isif_cfg.linear_tbl0_addr)
> +		iounmap(isif_cfg.linear_tbl0_addr);
> +
> +	while (i >= 0) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		release_mem_region(res->start, resource_size(res));
> +		i--;
> +	}
> +fail_mclk:
> +	clk_put(isif_cfg.mclk);
> +	vpfe_unregister_ccdc_device(&isif_hw_dev);
> +	return status;
> +}
> +
> +static int isif_remove(struct platform_device *pdev)
> +{
> +	struct resource	*res;
> +	int i = 0;
> +
> +	iounmap(isif_cfg.base_addr);
> +	iounmap(isif_cfg.linear_tbl0_addr);
> +	iounmap(isif_cfg.linear_tbl1_addr);
> +	while (i < 3) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> +		if (res)
> +			release_mem_region(res->start, resource_size(res));
> +		i++;
> +	}
> +	vpfe_unregister_ccdc_device(&isif_hw_dev);
> +	return 0;
> +}
> +
> +static struct platform_driver isif_driver = {
> +	.driver = {
> +		.name	= "isif",
> +		.owner = THIS_MODULE,
> +	},
> +	.remove = __devexit_p(isif_remove),
> +	.probe = isif_probe,
> +};
> +
> +static int __init isif_init(void)
> +{
> +	return platform_driver_register(&isif_driver);
> +}
> +
> +static void isif_exit(void)
> +{
> +	platform_driver_unregister(&isif_driver);
> +}
> +
> +module_init(isif_init);
> +module_exit(isif_exit);
> +
> +MODULE_LICENSE("GPL");
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-10 17:00         ` [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements m-karicheri2
@ 2009-12-15 21:05           ` Hans Verkuil
  2009-12-15 21:20             ` Hans Verkuil
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2009-12-15 21:05 UTC (permalink / raw)
  To: m-karicheri2
  Cc: linux-media, khilman, nsekhar, hvaibhav, davinci-linux-open-source

On Thursday 10 December 2009 18:00:29 m-karicheri2@ti.com wrote:
> From: Muralidharan Karicheri <m-karicheri2@ti.com>
> 
> Added a experimental IOCTL, to read the CCDC parameters.
> Default handler was not getting the original pointer from the core.
> So a wrapper function added to handle the default handler properly.
> Also fixed a bug in the probe() in the linux-next tree
> 
> Signed-off-by: Muralidharan Karicheri <m-karicheri2@ti.com>
> ---
>  drivers/media/video/davinci/vpfe_capture.c |  119 +++++++++++++++++-----------
>  include/media/davinci/vpfe_capture.h       |   12 ++-
>  2 files changed, 81 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/media/video/davinci/vpfe_capture.c b/drivers/media/video/davinci/vpfe_capture.c
> index 091750e..8c6d856 100644
> --- a/drivers/media/video/davinci/vpfe_capture.c
> +++ b/drivers/media/video/davinci/vpfe_capture.c
> @@ -759,12 +759,83 @@ static unsigned int vpfe_poll(struct file *file, poll_table *wait)
>  	return 0;
>  }
>  
> +static long vpfe_param_handler(struct file *file, void *priv,
> +		int cmd, void *param)
> +{
> +	struct vpfe_device *vpfe_dev = video_drvdata(file);
> +	int ret = 0;
> +
> +	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_param_handler\n");
> +
> +	if (NULL == param) {
> +		v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
> +			"Invalid user ptr\n");

Shouldn't there be an error return here? It looks weird.

> +	}
> +
> +	if (vpfe_dev->started) {
> +		/* only allowed if streaming is not started */
> +		v4l2_err(&vpfe_dev->v4l2_dev, "device already started\n");
> +		return -EBUSY;
> +	}
> +
> +
> +	switch (cmd) {
> +	case VPFE_CMD_S_CCDC_RAW_PARAMS:
> +		v4l2_warn(&vpfe_dev->v4l2_dev,
> +			  "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
> +		ret = mutex_lock_interruptible(&vpfe_dev->lock);
> +		if (ret)
> +			return ret;
> +		ret = ccdc_dev->hw_ops.set_params(param);
> +		if (ret) {
> +			v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
> +				"Error in setting parameters in CCDC\n");
> +			goto unlock_out;
> +		}
> +
> +		if (vpfe_get_ccdc_image_format(vpfe_dev, &vpfe_dev->fmt) < 0) {
> +			v4l2_err(&vpfe_dev->v4l2_dev,
> +				"Invalid image format at CCDC\n");
> +			ret = -EINVAL;
> +		}
> +unlock_out:
> +		mutex_unlock(&vpfe_dev->lock);
> +		break;
> +	case VPFE_CMD_G_CCDC_RAW_PARAMS:
> +		v4l2_warn(&vpfe_dev->v4l2_dev,
> +			  "VPFE_CMD_G_CCDC_RAW_PARAMS: experimental ioctl\n");
> +		if (!ccdc_dev->hw_ops.get_params) {
> +			ret = -EINVAL;
> +			break;
> +		}
> +		ret = ccdc_dev->hw_ops.get_params(param);
> +		if (ret) {
> +			v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev,
> +				"Error in getting parameters from CCDC\n");
> +		}
> +		break;
> +
> +	default:
> +		ret = -EINVAL;

Please add a break statement here.

> +	}
> +	return ret;
> +}
> +
> +static long vpfe_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	if (cmd == VPFE_CMD_S_CCDC_RAW_PARAMS ||
> +	    cmd == VPFE_CMD_G_CCDC_RAW_PARAMS)
> +		return vpfe_param_handler(file, file->private_data, cmd,
> +					 (void *)arg);
> +	return video_ioctl2(file, cmd, arg);
> +}
> +
>  /* vpfe capture driver file operations */
>  static const struct v4l2_file_operations vpfe_fops = {
>  	.owner = THIS_MODULE,
>  	.open = vpfe_open,
>  	.release = vpfe_release,
> -	.unlocked_ioctl = video_ioctl2,
> +	.unlocked_ioctl = vpfe_ioctl,
>  	.mmap = vpfe_mmap,
>  	.poll = vpfe_poll
>  };
> @@ -1682,50 +1753,6 @@ unlock_out:
>  	return ret;
>  }
>  
> -
> -static long vpfe_param_handler(struct file *file, void *priv,
> -		int cmd, void *param)
> -{
> -	struct vpfe_device *vpfe_dev = video_drvdata(file);
> -	int ret = 0;
> -
> -	v4l2_dbg(1, debug, &vpfe_dev->v4l2_dev, "vpfe_param_handler\n");
> -
> -	if (vpfe_dev->started) {
> -		/* only allowed if streaming is not started */
> -		v4l2_err(&vpfe_dev->v4l2_dev, "device already started\n");
> -		return -EBUSY;
> -	}
> -
> -	ret = mutex_lock_interruptible(&vpfe_dev->lock);
> -	if (ret)
> -		return ret;
> -
> -	switch (cmd) {
> -	case VPFE_CMD_S_CCDC_RAW_PARAMS:
> -		v4l2_warn(&vpfe_dev->v4l2_dev,
> -			  "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
> -		ret = ccdc_dev->hw_ops.set_params(param);
> -		if (ret) {
> -			v4l2_err(&vpfe_dev->v4l2_dev,
> -				"Error in setting parameters in CCDC\n");
> -			goto unlock_out;
> -		}
> -		if (vpfe_get_ccdc_image_format(vpfe_dev, &vpfe_dev->fmt) < 0) {
> -			v4l2_err(&vpfe_dev->v4l2_dev,
> -				"Invalid image format at CCDC\n");
> -			goto unlock_out;
> -		}
> -		break;
> -	default:
> -		ret = -EINVAL;
> -	}
> -unlock_out:
> -	mutex_unlock(&vpfe_dev->lock);
> -	return ret;
> -}
> -
> -
>  /* vpfe capture ioctl operations */
>  static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
>  	.vidioc_querycap	 = vpfe_querycap,
> @@ -1751,7 +1778,6 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
>  	.vidioc_cropcap		 = vpfe_cropcap,
>  	.vidioc_g_crop		 = vpfe_g_crop,
>  	.vidioc_s_crop		 = vpfe_s_crop,
> -	.vidioc_default		 = vpfe_param_handler,
>  };
>  
>  static struct vpfe_device *vpfe_initialize(void)
> @@ -1923,6 +1949,7 @@ static __init int vpfe_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, vpfe_dev);
>  	/* set driver private data */
>  	video_set_drvdata(vpfe_dev->video_dev, vpfe_dev);
> +	vpfe_cfg = pdev->dev.platform_data;
>  	i2c_adap = i2c_get_adapter(vpfe_cfg->i2c_adapter_id);
>  	num_subdevs = vpfe_cfg->num_subdevs;
>  	vpfe_dev->sd = kmalloc(sizeof(struct v4l2_subdev *) * num_subdevs,
> diff --git a/include/media/davinci/vpfe_capture.h b/include/media/davinci/vpfe_capture.h
> index d863e5e..23043bd 100644
> --- a/include/media/davinci/vpfe_capture.h
> +++ b/include/media/davinci/vpfe_capture.h
> @@ -31,8 +31,6 @@
>  #include <media/videobuf-dma-contig.h>
>  #include <media/davinci/vpfe_types.h>
>  
> -#define VPFE_CAPTURE_NUM_DECODERS        5
> -
>  /* Macros */
>  #define VPFE_MAJOR_RELEASE              0
>  #define VPFE_MINOR_RELEASE              0
> @@ -91,8 +89,9 @@ struct vpfe_config {
>  	char *card_name;
>  	/* ccdc name */
>  	char *ccdc;
> -	/* vpfe clock */
> +	/* vpfe clock. Obsolete! Will be removed in next patch */
>  	struct clk *vpssclk;
> +	/* Obsolete! Will be removed in next patch */
>  	struct clk *slaveclk;
>  };
>  
> @@ -193,8 +192,13 @@ struct vpfe_config_params {
>   * color space conversion, culling etc. This is an experimental ioctl that
>   * will change in future kernels. So use this ioctl with care !
>   * TODO: This is to be split into multiple ioctls and also explore the
> - * possibility of extending the v4l2 api to include this
> + * possibility of extending the v4l2 api to include this.
> + * VPFE_CMD_G_CCDC_RAW_PARAMS - EXPERIMENTAL IOCTL to get the current raw
> + * capture parameters
>   **/
>  #define VPFE_CMD_S_CCDC_RAW_PARAMS _IOW('V', BASE_VIDIOC_PRIVATE + 1, \
>  					void *)
> +#define VPFE_CMD_G_CCDC_RAW_PARAMS _IOR('V', BASE_VIDIOC_PRIVATE + 2, \
> +					void *)
> +
>  #endif				/* _DAVINCI_VPFE_H */
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-15 21:05           ` Hans Verkuil
@ 2009-12-15 21:20             ` Hans Verkuil
  2009-12-15 23:37               ` Karicheri, Muralidharan
  0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2009-12-15 21:20 UTC (permalink / raw)
  To: m-karicheri2
  Cc: linux-media, khilman, nsekhar, hvaibhav, davinci-linux-open-source

Note that the other patches from this series are fine as far as I am concerned.

One general note: I always have difficulties with constructions like this:

> +                     val = (bc->horz.win_count_calc &
> +                             ISIF_HORZ_BC_WIN_COUNT_MASK) |
> +                             ((!!bc->horz.base_win_sel_calc) <<
> +                             ISIF_HORZ_BC_WIN_SEL_SHIFT) |
> +                             ((!!bc->horz.clamp_pix_limit) <<
> +                             ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
> +                             ((bc->horz.win_h_sz_calc &
> +                             ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
> +                             ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
> +                             ((bc->horz.win_v_sz_calc &
> +                             ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
> +                             ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);

It's just about impossible for me to parse. And some of the patches in this
series are full of such constructs.

Unfortunately, I do not have a magic bullet solution. In some cases I suspect
that a static inline function can help.

In other cases it might help to split it up in smaller parts. For example:

u32 tmp;

val = bc->horz.win_count_calc &
	ISIF_HORZ_BC_WIN_COUNT_MASK;
val |= !!bc->horz.base_win_sel_calc <<
	ISIF_HORZ_BC_WIN_SEL_SHIFT;
val |= !!bc->horz.clamp_pix_limit <<
	ISIF_HORZ_BC_PIX_LIMIT_SHIFT;
tmp = bc->horz.win_h_sz_calc &
	ISIF_HORZ_BC_WIN_H_SIZE_MASK;
val |= tmp << ISIF_HORZ_BC_WIN_H_SIZE_SHIFT;
tmp = bc->horz.win_v_sz_calc &
	ISIF_HORZ_BC_WIN_V_SIZE_MASK;
val |= tmp << ISIF_HORZ_BC_WIN_V_SIZE_SHIFT;

Of course, in this particular piece of code from the function isif_config_bclamp()
I am also wondering why bc->horz.win_h_sz_calc and bc->horz.win_v_sz_calc need to
be ANDed anyway. I would expect that to happen when these values are set. But I
did not look at this in-depth, so I may well have missed some subtlety here.

It would be interesting to know if people know of good ways of making awkward
code like this more elegant (or at least less awkward).

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-15 21:20             ` Hans Verkuil
@ 2009-12-15 23:37               ` Karicheri, Muralidharan
  2009-12-16  7:41                 ` Hans Verkuil
  0 siblings, 1 reply; 17+ messages in thread
From: Karicheri, Muralidharan @ 2009-12-15 23:37 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, khilman, Nori, Sekhar, Hiremath, Vaibhav,
	davinci-linux-open-source

Hans,

I remember there was a comment against an earlier patch that asks
for combining such statements since it makes the function appear
as big. Not sure who had made that comment. That is the reason you
find code like this in this patch. It was initially done with multiple
OR statements to construct the value to be written to the register as you stated below as 

>val = bc->horz.win_count_calc &
>	ISIF_HORZ_BC_WIN_COUNT_MASK;
>val |= !!bc->horz.base_win_sel_calc <<
>	ISIF_HORZ_BC_WIN_SEL_SHIFT;

I have checked few other drivers such as bt819.c ir-kbd-i2c.c,
mt9m111.c etc, where similar statements are found, but they have used hardcoded values masks which makes it appears less complex. But I 
like to reduce magic numbers as much possible in the code.

I think what I can do is to  combine maximum of 2 such expressions in a statement which might make it less complex to read. Such as 

val = (bc->horz.win_count_calc &
	ISIF_HORZ_BC_WIN_COUNT_MASK) |
 	((!!bc->horz.base_win_sel_calc) <<
	ISIF_HORZ_BC_WIN_SEL_SHIFT);

val |= (!!bc->horz.clamp_pix_limit) <<
	 ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
 	 ((bc->horz.win_h_sz_calc &
	 ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
	 ISIF_HORZ_BC_WIN_H_SIZE_SHIFT);
val |= ((bc->horz.win_v_sz_calc &
	 ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
	 ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);

Also to make the line fits in 80 characters, I will consider reducing
the number of characters in #define names such as

val = (bc->horz.win_count_calc & HZ_BC_WIN_CNT_MASK) |
((!!bc->horz.base_win_sel_calc) << HZ_BC_WIN_SEL_SHIFT) |
(!!bc->horz.clamp_pix_limit) << HZ_BC_PIX_LIMIT_SHIFT);

Let me know if you don't agree.

>
>Of course, in this particular piece of code from the function
>isif_config_bclamp()
>I am also wondering why bc->horz.win_h_sz_calc and bc->horz.win_v_sz_calc
>need to
>be ANDed anyway. I would expect that to happen when these values are set.
>But I
>did not look at this in-depth, so I may well have missed some subtlety here.

Yes, isif_config_bclamp() set values in the register.

>
>It would be interesting to know if people know of good ways of making
>awkward
>code like this more elegant (or at least less awkward).
>
>Regards,
>
>	Hans
>
>--
>Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-15 23:37               ` Karicheri, Muralidharan
@ 2009-12-16  7:41                 ` Hans Verkuil
  2009-12-16 16:45                   ` Karicheri, Muralidharan
  2009-12-18  5:13                   ` Nori, Sekhar
  0 siblings, 2 replies; 17+ messages in thread
From: Hans Verkuil @ 2009-12-16  7:41 UTC (permalink / raw)
  To: Karicheri, Muralidharan
  Cc: linux-media, khilman, Nori, Sekhar, Hiremath, Vaibhav,
	davinci-linux-open-source

On Wednesday 16 December 2009 00:37:52 Karicheri, Muralidharan wrote:
> Hans,
> 
> I remember there was a comment against an earlier patch that asks
> for combining such statements since it makes the function appear
> as big. Not sure who had made that comment. That is the reason you
> find code like this in this patch. It was initially done with multiple
> OR statements to construct the value to be written to the register as you stated below as 
> 
> >val = bc->horz.win_count_calc &
> >	ISIF_HORZ_BC_WIN_COUNT_MASK;
> >val |= !!bc->horz.base_win_sel_calc <<
> >	ISIF_HORZ_BC_WIN_SEL_SHIFT;
> 
> I have checked few other drivers such as bt819.c ir-kbd-i2c.c,
> mt9m111.c etc, where similar statements are found, but they have used hardcoded values masks which makes it appears less complex. But I 
> like to reduce magic numbers as much possible in the code.

Personally I have mixed feelings about the use for symbolic names for things
like this. The problem is that they do not help me understanding the code.
The names tend to be long, leading to these broken up lines, and if I want
to know how the bits are distributed in the value I continuously have to
do the look up in the header containing these defines.

Frankly, I have a similar problem with using symbolic names for registers.
Every time I need to look up a register in the datasheet I first need to
look up the register number the register name maps to. All datasheets seem
to be organized around the register addresses and there rarely is a datasheet
that has an index of symbolic names.

Using hard-coded numbers together with a well written comment tends to be much
more readable in my opinion. I don't really think there is anything magic about
these numbers: these are exactly the numbers that I need to know whenever I
have to deal with the datasheet. Having to go through a layer of obfuscation
is just plain irritating to me.

However, I seem to be a minority when it comes to this and I've given up
arguing for this...

Note that all this assumes that the datasheet is publicly available. If it
is a reversed engineered driver or based on NDA datasheets only, then the
symbolic names may be all there is to make you understand what is going on.

> 
> I think what I can do is to  combine maximum of 2 such expressions in a statement which might make it less complex to read. Such as 
> 
> val = (bc->horz.win_count_calc &
> 	ISIF_HORZ_BC_WIN_COUNT_MASK) |
>  	((!!bc->horz.base_win_sel_calc) <<
> 	ISIF_HORZ_BC_WIN_SEL_SHIFT);
> 
> val |= (!!bc->horz.clamp_pix_limit) <<
> 	 ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
>  	 ((bc->horz.win_h_sz_calc &
> 	 ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
> 	 ISIF_HORZ_BC_WIN_H_SIZE_SHIFT);
> val |= ((bc->horz.win_v_sz_calc &
> 	 ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
> 	 ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
> 
> Also to make the line fits in 80 characters, I will consider reducing
> the number of characters in #define names such as
> 
> val = (bc->horz.win_count_calc & HZ_BC_WIN_CNT_MASK) |
> ((!!bc->horz.base_win_sel_calc) << HZ_BC_WIN_SEL_SHIFT) |
> (!!bc->horz.clamp_pix_limit) << HZ_BC_PIX_LIMIT_SHIFT);
> 
> Let me know if you don't agree.

That seems overkill. I actually think it can be improved a lot by visually
grouping the lines:

                     val = (bc->horz.win_count_calc &
                             ISIF_HORZ_BC_WIN_COUNT_MASK) |
                           ((!!bc->horz.base_win_sel_calc) <<
                             ISIF_HORZ_BC_WIN_SEL_SHIFT) |
                           ((!!bc->horz.clamp_pix_limit) <<
                             ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
                           ((bc->horz.win_h_sz_calc &
                             ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
                             ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
                           ((bc->horz.win_v_sz_calc &
                             ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
                             ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);

Now I can at least see the various values that are ORed.

> 
> >
> >Of course, in this particular piece of code from the function
> >isif_config_bclamp()
> >I am also wondering why bc->horz.win_h_sz_calc and bc->horz.win_v_sz_calc
> >need to
> >be ANDed anyway. I would expect that to happen when these values are set.
> >But I
> >did not look at this in-depth, so I may well have missed some subtlety here.
> 
> Yes, isif_config_bclamp() set values in the register.

Huh? That does not explain why apparently bc->horz.win_h_sz_calc can be larger
than ISIF_HORZ_BC_WIN_H_SIZE_MASK.

Regards,

	Hans

> 
> >
> >It would be interesting to know if people know of good ways of making
> >awkward
> >code like this more elegant (or at least less awkward).
> >
> >Regards,
> >
> >	Hans
> >
> >--
> >Hans Verkuil - video4linux developer - sponsored by TANDBERG
> 
> 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-16  7:41                 ` Hans Verkuil
@ 2009-12-16 16:45                   ` Karicheri, Muralidharan
  2009-12-18  5:13                   ` Nori, Sekhar
  1 sibling, 0 replies; 17+ messages in thread
From: Karicheri, Muralidharan @ 2009-12-16 16:45 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, khilman, Nori, Sekhar, Hiremath, Vaibhav,
	davinci-linux-open-source

hans,

>>
>> Yes, isif_config_bclamp() set values in the register.
>
>Huh? That does not explain why apparently bc->horz.win_h_sz_calc can be
>larger
>than ISIF_HORZ_BC_WIN_H_SIZE_MASK.
because the values come from the user and since we can't use the enum
for the types, I have to make sure the value is within range. Other way
to do is to check the value in the validate() function. I am inclined to
do the validation so that the & statements with masks can be removed while setting it in the register.

>
>Regards,
>
>	Hans
>
>>
>> >
>> >It would be interesting to know if people know of good ways of making
>> >awkward
>> >code like this more elegant (or at least less awkward).
>> >
>> >Regards,
>> >
>> >	Hans
>> >
>> >--
>> >Hans Verkuil - video4linux developer - sponsored by TANDBERG
>>
>>
>
>--
>Hans Verkuil - video4linux developer - sponsored by TANDBERG

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files
  2009-12-15  7:46   ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files Hans Verkuil
@ 2009-12-17 21:34     ` Karicheri, Muralidharan
  0 siblings, 0 replies; 17+ messages in thread
From: Karicheri, Muralidharan @ 2009-12-17 21:34 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, khilman, Nori, Sekhar, Hiremath, Vaibhav,
	davinci-linux-open-source

Hans,

Thanks for reviewing this.

>
>> +/* isif float type S8Q8/U8Q8 */
>> +struct isif_float_8 {
>> +	/* 8 bit integer part */
>> +	__u8 integer;
>> +	/* 8 bit decimal part */
>> +	__u8 decimal;
>
>Why __u8 instead of u8? This is a kernel-internal file, so it does not need
>to use the '__' variants.
>
This is a public header available for configuring the isif ip blocks such
as black clamp, defect correction, linearization etc. In future once
sub devices have its own node, application will direct the ioctls to
the sub device node. But as of now, it is going through video node.
I can think of splitting this into 2 files, isif.h that contains the
kernel part and isif-ioctl.h that will have structure and types for ioctls.
But for DM355 & DM644x ccdc, this is how it is done.

>If this needs to be publicly available, then it should be in include/linux.
Currently all of dm355 and dm644x header files for user space use are
under include/media/davinci. To me it makes sense since only davinci
specific applications will ever need to include them. I know you have
ivtv.h under linux/. So also several user space header files under
include/media and include/video. So not sure if there is a strict
rule that is followed. 

To a different note, Vaibhav had sent a patch for re-arranging the
driver files in a different folder to allow re-use of drivers
across DaVinci/OMAP devices. The proposal was to move all of TI
driver source files to

drivers/media/video/ti-media

and include files to
include/media/ti-media

What you think about this proposal? So at this point, shall we keep
it in it's current location?

>The comment before each shift define seems rather pointless.
>
>> +	/* Data shift applied before storing to SDRAM */
>> +	__u8 data_shift;
>> +	/* enable input test pattern generation */
>> +	__u8 test_pat_gen;
>> +};
>> +
>> +#ifdef __KERNEL__
>
>Hmm. Is this header supposed to be public or private to the kernel?
>

Actually it has both as is the case with dm355 & dm644x ccdc headers.
Do you want me to split the headers as suggested earlier?

Thanks

Murali

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements
  2009-12-16  7:41                 ` Hans Verkuil
  2009-12-16 16:45                   ` Karicheri, Muralidharan
@ 2009-12-18  5:13                   ` Nori, Sekhar
  1 sibling, 0 replies; 17+ messages in thread
From: Nori, Sekhar @ 2009-12-18  5:13 UTC (permalink / raw)
  To: Hans Verkuil, Karicheri, Muralidharan
  Cc: linux-media, khilman, Hiremath, Vaibhav, davinci-linux-open-source

On Wed, Dec 16, 2009 at 13:11:57, Hans Verkuil wrote:
> On Wednesday 16 December 2009 00:37:52 Karicheri, Muralidharan wrote:
> > Hans,
> >
> > I remember there was a comment against an earlier patch that asks
> > for combining such statements since it makes the function appear
> > as big. Not sure who had made that comment. That is the reason you
> > find code like this in this patch. It was initially done with multiple
> > OR statements to construct the value to be written to the register as you stated below as
> >
> > >val = bc->horz.win_count_calc &
> > >   ISIF_HORZ_BC_WIN_COUNT_MASK;
> > >val |= !!bc->horz.base_win_sel_calc <<
> > >   ISIF_HORZ_BC_WIN_SEL_SHIFT;
> >
> > I have checked few other drivers such as bt819.c ir-kbd-i2c.c,
> > mt9m111.c etc, where similar statements are found, but they have used hardcoded values masks which makes it appears less complex. But I
> > like to reduce magic numbers as much possible in the code.
>
> Personally I have mixed feelings about the use for symbolic names for things
> like this. The problem is that they do not help me understanding the code.
> The names tend to be long, leading to these broken up lines, and if I want
> to know how the bits are distributed in the value I continuously have to
> do the look up in the header containing these defines.
>
> Frankly, I have a similar problem with using symbolic names for registers.
> Every time I need to look up a register in the datasheet I first need to
> look up the register number the register name maps to. All datasheets seem
> to be organized around the register addresses and there rarely is a datasheet
> that has an index of symbolic names.
>
> Using hard-coded numbers together with a well written comment tends to be much
> more readable in my opinion. I don't really think there is anything magic about
> these numbers: these are exactly the numbers that I need to know whenever I
> have to deal with the datasheet. Having to go through a layer of obfuscation
> is just plain irritating to me.
>
> However, I seem to be a minority when it comes to this and I've given up
> arguing for this...
>
> Note that all this assumes that the datasheet is publicly available. If it
> is a reversed engineered driver or based on NDA datasheets only, then the
> symbolic names may be all there is to make you understand what is going on.
>

[...]

>
> That seems overkill. I actually think it can be improved a lot by visually
> grouping the lines:
>
>                      val = (bc->horz.win_count_calc &
>                              ISIF_HORZ_BC_WIN_COUNT_MASK) |
>                            ((!!bc->horz.base_win_sel_calc) <<
>                              ISIF_HORZ_BC_WIN_SEL_SHIFT) |
>                            ((!!bc->horz.clamp_pix_limit) <<
>                              ISIF_HORZ_BC_PIX_LIMIT_SHIFT) |
>                            ((bc->horz.win_h_sz_calc &
>                              ISIF_HORZ_BC_WIN_H_SIZE_MASK) <<
>                              ISIF_HORZ_BC_WIN_H_SIZE_SHIFT) |
>                            ((bc->horz.win_v_sz_calc &
>                              ISIF_HORZ_BC_WIN_V_SIZE_MASK) <<
>                              ISIF_HORZ_BC_WIN_V_SIZE_SHIFT);
>
> Now I can at least see the various values that are ORed.
>

I liked this piece of code from drivers/mtd/nand/s3c2410.c. Serves as
a good template to do this sort of thing.

#define S3C2410_NFCONF_TACLS(x)    ((x)<<8)
#define S3C2410_NFCONF_TWRPH0(x)   ((x)<<4)
#define S3C2410_NFCONF_TWRPH1(x)   ((x)<<0)

[Okay, spaces around '<<', please :)]

[...]

        if (plat != NULL) {
                tacls = s3c_nand_calc_rate(plat->tacls, clkrate, tacls_max);
                twrph0 = s3c_nand_calc_rate(plat->twrph0, clkrate, 8);
                twrph1 = s3c_nand_calc_rate(plat->twrph1, clkrate, 8);
        }

[...]

                mask = (S3C2410_NFCONF_TACLS(3) |
                        S3C2410_NFCONF_TWRPH0(7) |
                        S3C2410_NFCONF_TWRPH1(7));
                set = S3C2410_NFCONF_EN;
                set |= S3C2410_NFCONF_TACLS(tacls - 1);
                set |= S3C2410_NFCONF_TWRPH0(twrph0 - 1);
                set |= S3C2410_NFCONF_TWRPH1(twrph1 - 1);

[...]

        cfg = readl(info->regs + S3C2410_NFCONF);
        cfg &= ~mask;
        cfg |= set;
        writel(cfg, info->regs + S3C2410_NFCONF);

And Murali said:

> >Huh? That does not explain why apparently bc->horz.win_h_sz_calc can be
> >larger
> >than ISIF_HORZ_BC_WIN_H_SIZE_MASK.
> because the values come from the user and since we can't use the enum
> for the types, I have to make sure the value is within range. Other way
> to do is to check the value in the validate() function. I am inclined to
> do the validation so that the & statements with masks can be removed while setting it in
> the register.

Agree fully here. Either a separate validate function or
an if check before using the values. Results with masking
or without masking are both unpredictable.

Thanks,
Sekhar


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-12-18  5:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-10 17:00 [PATCH - v1 6/6] DaVinci - Adding support for vpfe capture on DM365 m-karicheri2
2009-12-10 17:00 ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files m-karicheri2
2009-12-10 17:00   ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source m-karicheri2
2009-12-10 17:00     ` [PATCH - v1 1/6] V4L - vpfe-capture : DM365 vpss enhancements m-karicheri2
2009-12-10 17:00       ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver m-karicheri2
2009-12-10 17:00         ` [PATCH - v1 4/6] V4L - vpfe_capture bug fix and enhancements m-karicheri2
2009-12-15 21:05           ` Hans Verkuil
2009-12-15 21:20             ` Hans Verkuil
2009-12-15 23:37               ` Karicheri, Muralidharan
2009-12-16  7:41                 ` Hans Verkuil
2009-12-16 16:45                   ` Karicheri, Muralidharan
2009-12-18  5:13                   ` Nori, Sekhar
2009-12-10 17:38         ` [PATCH - v1 5/6] V4L - vpfe capture - build environment for ISIF driver Sergei Shtylyov
2009-12-11 20:50           ` Karicheri, Muralidharan
2009-12-15  7:57     ` [PATCH - v1 3/6] V4L-vpfe-capture-Adding ISIF driver for DM365 - source Hans Verkuil
2009-12-15  7:46   ` [PATCH - v1 2/6] V4L - vpfe capture - Adding DM365 ISIF driver - header files Hans Verkuil
2009-12-17 21:34     ` Karicheri, Muralidharan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.