[PATCH 0/4] davinci: add vpif support for da850/omap-l138

[PATCH 0/4] davinci: add vpif support for da850/omap-l138

[Manjunath Hadli]

add vpif capture and display driver support for
da850/omap-l138 by taking care of the interrupt 
behavior changes and removing platform specific
connotations.

Manjunath Hadli (4):
  davinci: vpif: add check for genuine interrupts in the isr
  davinci: vpif: make generic changes to re-use the vpif drivers on
    da850/omap-l138 soc
  davinci: vpif: make request_irq flags as shared
  davinci: da850: add build configuration for vpif drivers

 drivers/media/video/davinci/Kconfig        |   26 +++++++++++++++++++++++++-
 drivers/media/video/davinci/Makefile       |    5 +++++
 drivers/media/video/davinci/vpif.c         |    2 +-
 drivers/media/video/davinci/vpif_capture.c |   16 ++++++++++------
 drivers/media/video/davinci/vpif_display.c |   21 +++++++++++++--------
 drivers/media/video/davinci/vpif_display.h |    2 +-
 include/media/davinci/vpif_types.h         |    2 ++
 7 files changed, 57 insertions(+), 17 deletions(-)

[PATCH 1/4] davinci: vpif: add check for genuine interrupts in the isr

[Manjunath Hadli]

add a condition to in the isr to check for interrupt ownership and
channel number to make sure we do not service wrong interrupts.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
---
 drivers/media/video/davinci/vpif_capture.c |    5 +++++
 drivers/media/video/davinci/vpif_display.c |    5 +++++
 include/media/davinci/vpif_types.h         |    2 ++
 3 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
index 6504e40..33d865d 100644
--- a/drivers/media/video/davinci/vpif_capture.c
+++ b/drivers/media/video/davinci/vpif_capture.c
@@ -333,6 +333,7 @@ static void vpif_schedule_next_buffer(struct common_obj *common)
  */
 static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
 {
+	struct vpif_capture_config *config = vpif_dev->platform_data;
 	struct vpif_device *dev = &vpif_obj;
 	struct common_obj *common;
 	struct channel_obj *ch;
@@ -341,6 +342,10 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
 	int fid = -1, i;
 
 	channel_id = *(int *)(dev_id);
+	if (!config->intr_status ||
+			!config->intr_status(vpif_base, channel_id))
+		return IRQ_NONE;
+
 	ch = dev->dev[channel_id];
 
 	field = ch->common[VPIF_VIDEO_INDEX].fmt.fmt.pix.field;
diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
index 286f029..b315ccf 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -301,6 +301,7 @@ static void process_interlaced_mode(int fid, struct common_obj *common)
  */
 static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
 {
+	struct vpif_display_config *config = vpif_dev->platform_data;
 	struct vpif_device *dev = &vpif_obj;
 	struct channel_obj *ch;
 	struct common_obj *common;
@@ -309,6 +310,10 @@ static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
 	int channel_id = 0;
 
 	channel_id = *(int *)(dev_id);
+	if (!config->intr_status ||
+		!config->intr_status(vpif_base, channel_id + 2))
+		return IRQ_NONE;
+
 	ch = dev->dev[channel_id];
 	field = ch->common[VPIF_VIDEO_INDEX].fmt.fmt.pix.field;
 	for (i = 0; i < VPIF_NUMOBJECTS; i++) {
diff --git a/include/media/davinci/vpif_types.h b/include/media/davinci/vpif_types.h
index 9929b05..4802d06 100644
--- a/include/media/davinci/vpif_types.h
+++ b/include/media/davinci/vpif_types.h
@@ -43,6 +43,7 @@ struct vpif_subdev_info {
 
 struct vpif_display_config {
 	int (*set_clock)(int, int);
+	int (*intr_status)(void __iomem *vpif_base, int);
 	struct vpif_subdev_info *subdevinfo;
 	int subdev_count;
 	const char **output;
@@ -63,6 +64,7 @@ struct vpif_capture_chan_config {
 struct vpif_capture_config {
 	int (*setup_input_channel_mode)(int);
 	int (*setup_input_path)(int, const char *);
+	int (*intr_status)(void __iomem *vpif_base, int);
 	struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
 	struct vpif_subdev_info *subdev_info;
 	int subdev_count;
-- 
1.6.2.4

[PATCH 2/4] davinci: vpif: make generic changes to re-use the vpif drivers on da850/omap-l138 soc

[Manjunath Hadli]

change the dm646x specific strings in the driver to make
them generic across platforms. In this case change all the
strings which have a dm646x connotation to vpif which is a
platform independent ip.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
---
 drivers/media/video/davinci/vpif.c         |    2 +-
 drivers/media/video/davinci/vpif_capture.c |    9 ++++-----
 drivers/media/video/davinci/vpif_display.c |   14 +++++++-------
 drivers/media/video/davinci/vpif_display.h |    2 +-
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/media/video/davinci/vpif.c b/drivers/media/video/davinci/vpif.c
index af96802..774bcd3 100644
--- a/drivers/media/video/davinci/vpif.c
+++ b/drivers/media/video/davinci/vpif.c
@@ -1,5 +1,5 @@
 /*
- * vpif - DM646x Video Port Interface driver
+ * vpif - Video Port Interface driver
  * VPIF is a receiver and transmitter for video data. It has two channels(0, 1)
  * that receiveing video byte stream and two channels(2, 3) for video output.
  * The hardware supports SDTV, HDTV formats, raw data capture.
diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
index 33d865d..010cce4 100644
--- a/drivers/media/video/davinci/vpif_capture.c
+++ b/drivers/media/video/davinci/vpif_capture.c
@@ -1684,7 +1684,7 @@ static int vpif_querycap(struct file *file, void  *priv,
 
 	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING;
 	strlcpy(cap->driver, "vpif capture", sizeof(cap->driver));
-	strlcpy(cap->bus_info, "DM646x Platform", sizeof(cap->bus_info));
+	strlcpy(cap->bus_info, "VPIF Platform", sizeof(cap->bus_info));
 	strlcpy(cap->card, config->card_name, sizeof(cap->card));
 
 	return 0;
@@ -2192,7 +2192,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
 			if (request_irq(i, vpif_channel_isr, IRQF_DISABLED,
-					"DM646x_Capture",
+					"VPIF_Capture",
 				(void *)(&vpif_obj.dev[k]->channel_id))) {
 				err = -EBUSY;
 				i--;
@@ -2221,7 +2221,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 		vfd->v4l2_dev = &vpif_obj.v4l2_dev;
 		vfd->release = video_device_release;
 		snprintf(vfd->name, sizeof(vfd->name),
-			 "DM646x_VPIFCapture_DRIVER_V%s",
+			 "VPIF_Capture_DRIVER_V%s",
 			 VPIF_CAPTURE_VERSION);
 		/* Set video_dev to the video device */
 		ch->video_dev = vfd;
@@ -2276,8 +2276,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 			vpif_obj.sd[i]->grp_id = 1 << i;
 	}
 
-	v4l2_info(&vpif_obj.v4l2_dev,
-			"DM646x VPIF capture driver initialized\n");
+	v4l2_info(&vpif_obj.v4l2_dev, "VPIF capture driver initialized\n");
 	return 0;
 
 probe_subdev_out:
diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
index b315ccf..6f3eabb 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -48,7 +48,7 @@ MODULE_DESCRIPTION("TI DaVinci VPIF Display driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(VPIF_DISPLAY_VERSION);
 
-#define DM646X_V4L2_STD (V4L2_STD_525_60 | V4L2_STD_625_50)
+#define VPIF_V4L2_STD (V4L2_STD_525_60 | V4L2_STD_625_50)
 
 #define vpif_err(fmt, arg...)	v4l2_err(&vpif_obj.v4l2_dev, fmt, ## arg)
 #define vpif_dbg(level, debug, fmt, arg...)	\
@@ -976,7 +976,7 @@ static int vpif_s_std(struct file *file, void *priv, v4l2_std_id *std_id)
 	struct common_obj *common = &ch->common[VPIF_VIDEO_INDEX];
 	int ret = 0;
 
-	if (!(*std_id & DM646X_V4L2_STD))
+	if (!(*std_id & VPIF_V4L2_STD))
 		return -EINVAL;
 
 	if (common->started) {
@@ -1227,7 +1227,7 @@ static int vpif_enum_output(struct file *file, void *fh,
 
 	strcpy(output->name, config->output[output->index]);
 	output->type = V4L2_OUTPUT_TYPE_ANALOG;
-	output->std = DM646X_V4L2_STD;
+	output->std = VPIF_V4L2_STD;
 
 	return 0;
 }
@@ -1612,7 +1612,7 @@ static struct video_device vpif_video_template = {
 	.name		= "vpif",
 	.fops		= &vpif_fops,
 	.ioctl_ops	= &vpif_ioctl_ops,
-	.tvnorms	= DM646X_V4L2_STD,
+	.tvnorms	= VPIF_V4L2_STD,
 	.current_norm	= V4L2_STD_625_50,
 
 };
@@ -1714,7 +1714,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
 			if (request_irq(i, vpif_channel_isr, IRQF_DISABLED,
-					"DM646x_Display",
+					"VPIF_Display",
 				(void *)(&vpif_obj.dev[k]->channel_id))) {
 				err = -EBUSY;
 				goto vpif_int_err;
@@ -1744,7 +1744,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 		vfd->v4l2_dev = &vpif_obj.v4l2_dev;
 		vfd->release = video_device_release;
 		snprintf(vfd->name, sizeof(vfd->name),
-			 "DM646x_VPIFDisplay_DRIVER_V%s",
+			 "VPIF_Display_DRIVER_V%s",
 			 VPIF_DISPLAY_VERSION);
 
 		/* Set video_dev to the video device */
@@ -1826,7 +1826,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	}
 
 	v4l2_info(&vpif_obj.v4l2_dev,
-			"DM646x VPIF display driver initialized\n");
+			" VPIF display driver initialized\n");
 	return 0;
 
 probe_subdev_out:
diff --git a/drivers/media/video/davinci/vpif_display.h b/drivers/media/video/davinci/vpif_display.h
index 56879d1..dd4887c 100644
--- a/drivers/media/video/davinci/vpif_display.h
+++ b/drivers/media/video/davinci/vpif_display.h
@@ -1,5 +1,5 @@
 /*
- * DM646x display header file
+ * VPIF display header file
  *
  * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/
  *
-- 
1.6.2.4

[PATCH 3/4] davinci: vpif: make request_irq flags as shared

[Manjunath Hadli]

omap-l138 shares the interrupt between capture and display.
Make sure we are able to request for the same irq number
by making a shared irq request.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
---
 drivers/media/video/davinci/vpif_capture.c |    2 +-
 drivers/media/video/davinci/vpif_display.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c
index 010cce4..ad933e0 100644
--- a/drivers/media/video/davinci/vpif_capture.c
+++ b/drivers/media/video/davinci/vpif_capture.c
@@ -2191,7 +2191,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	k = 0;
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
-			if (request_irq(i, vpif_channel_isr, IRQF_DISABLED,
+			if (request_irq(i, vpif_channel_isr, IRQF_SHARED,
 					"VPIF_Capture",
 				(void *)(&vpif_obj.dev[k]->channel_id))) {
 				err = -EBUSY;
diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c
index 6f3eabb..c876c10 100644
--- a/drivers/media/video/davinci/vpif_display.c
+++ b/drivers/media/video/davinci/vpif_display.c
@@ -1713,7 +1713,7 @@ static __init int vpif_probe(struct platform_device *pdev)
 	k = 0;
 	while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, k))) {
 		for (i = res->start; i <= res->end; i++) {
-			if (request_irq(i, vpif_channel_isr, IRQF_DISABLED,
+			if (request_irq(i, vpif_channel_isr, IRQF_SHARED,
 					"VPIF_Display",
 				(void *)(&vpif_obj.dev[k]->channel_id))) {
 				err = -EBUSY;
-- 
1.6.2.4

[PATCH 4/4] davinci: da850: add build configuration for vpif drivers

[Manjunath Hadli]

add build configuration for da850/omapl-138 for vpif
capture and display drivers.

Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
---
 drivers/media/video/davinci/Kconfig  |   26 +++++++++++++++++++++++++-
 drivers/media/video/davinci/Makefile |    5 +++++
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/davinci/Kconfig b/drivers/media/video/davinci/Kconfig
index 60a456e..a0b0fb3 100644
--- a/drivers/media/video/davinci/Kconfig
+++ b/drivers/media/video/davinci/Kconfig
@@ -22,9 +22,33 @@ config CAPTURE_DAVINCI_DM646X_EVM
 	  To compile this driver as a module, choose M here: the
 	  module will be called vpif_capture.
 
+config DISPLAY_DAVINCI_DA850_EVM
+	tristate "DA850/OMAPL138 EVM Video Display"
+	depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM
+	select VIDEOBUF_DMA_CONTIG
+	select VIDEO_DAVINCI_VPIF
+	select VIDEO_ADV7343
+	select VIDEO_THS7303
+	help
+	  Support for DA850/OMAP-L138/AM18xx  based display device.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called vpif_display.
+
+config CAPTURE_DAVINCI_DA850_EVM
+	tristate "DA850/OMAPL138 EVM Video Capture"
+	depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM
+	select VIDEOBUF_DMA_CONTIG
+	select VIDEO_DAVINCI_VPIF
+	help
+	  Support for DA850/OMAP-L138/AM18xx  based capture device.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called vpif_capture.
+
 config VIDEO_DAVINCI_VPIF
 	tristate "DaVinci VPIF Driver"
-	depends on DISPLAY_DAVINCI_DM646X_EVM
+	depends on DISPLAY_DAVINCI_DM646X_EVM || DISPLAY_DAVINCI_DA850_EVM
 	help
 	  Support for DaVinci VPIF Driver.
 
diff --git a/drivers/media/video/davinci/Makefile b/drivers/media/video/davinci/Makefile
index ae7dafb..2c7cfb0 100644
--- a/drivers/media/video/davinci/Makefile
+++ b/drivers/media/video/davinci/Makefile
@@ -10,6 +10,11 @@ obj-$(CONFIG_DISPLAY_DAVINCI_DM646X_EVM) += vpif_display.o
 #DM646x EVM Capture driver
 obj-$(CONFIG_CAPTURE_DAVINCI_DM646X_EVM) += vpif_capture.o
 
+#DA850 EVM Display driver
+obj-$(CONFIG_DISPLAY_DAVINCI_DA850_EVM) += vpif_display.o
+#DA850 EVM Capture driver
+obj-$(CONFIG_CAPTURE_DAVINCI_DA850_EVM) += vpif_capture.o
+
 # Capture: DM6446 and DM355
 obj-$(CONFIG_VIDEO_VPSS_SYSTEM) += vpss.o
 obj-$(CONFIG_VIDEO_VPFE_CAPTURE) += vpfe_capture.o
-- 
1.6.2.4

RE: [PATCH 4/4] davinci: da850: add build configuration for vpif drivers

[Nori, Sekhar]

Hi Manju,

On Wed, Jan 25, 2012 at 20:35:34, Hadli, Manjunath wrote:
> add build configuration for da850/omapl-138 for vpif
> capture and display drivers.
> 
> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
> ---
>  drivers/media/video/davinci/Kconfig  |   26 +++++++++++++++++++++++++-
>  drivers/media/video/davinci/Makefile |    5 +++++
>  2 files changed, 30 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/video/davinci/Kconfig b/drivers/media/video/davinci/Kconfig
> index 60a456e..a0b0fb3 100644
> --- a/drivers/media/video/davinci/Kconfig
> +++ b/drivers/media/video/davinci/Kconfig
> @@ -22,9 +22,33 @@ config CAPTURE_DAVINCI_DM646X_EVM
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called vpif_capture.
>  
> +config DISPLAY_DAVINCI_DA850_EVM
> +	tristate "DA850/OMAPL138 EVM Video Display"
> +	depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM
> +	select VIDEOBUF_DMA_CONTIG
> +	select VIDEO_DAVINCI_VPIF
> +	select VIDEO_ADV7343
> +	select VIDEO_THS7303

Selecting these helper chips should be conditional on
VIDEO_HELPER_CHIPS_AUTO. This is what I gather looking
at existing Kconfig files like drivers/media/video/em28xx/Kconfig.

This issue exists with existing code too, so that should
be addressed separately.

> +	help
> +	  Support for DA850/OMAP-L138/AM18xx  based display device.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called vpif_display.
> +
> +config CAPTURE_DAVINCI_DA850_EVM
> +	tristate "DA850/OMAPL138 EVM Video Capture"
> +	depends on DA850_UI_SD_VIDEO_PORT && VIDEO_DEV && MACH_DAVINCI_DA850_EVM
> +	select VIDEOBUF_DMA_CONTIG
> +	select VIDEO_DAVINCI_VPIF
> +	help
> +	  Support for DA850/OMAP-L138/AM18xx  based capture device.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called vpif_capture.
> +
>  config VIDEO_DAVINCI_VPIF
>  	tristate "DaVinci VPIF Driver"
> -	depends on DISPLAY_DAVINCI_DM646X_EVM
> +	depends on DISPLAY_DAVINCI_DM646X_EVM || DISPLAY_DAVINCI_DA850_EVM
>  	help
>  	  Support for DaVinci VPIF Driver.
>  
> diff --git a/drivers/media/video/davinci/Makefile b/drivers/media/video/davinci/Makefile
> index ae7dafb..2c7cfb0 100644
> --- a/drivers/media/video/davinci/Makefile
> +++ b/drivers/media/video/davinci/Makefile
> @@ -10,6 +10,11 @@ obj-$(CONFIG_DISPLAY_DAVINCI_DM646X_EVM) += vpif_display.o
>  #DM646x EVM Capture driver
>  obj-$(CONFIG_CAPTURE_DAVINCI_DM646X_EVM) += vpif_capture.o
>  
> +#DA850 EVM Display driver
> +obj-$(CONFIG_DISPLAY_DAVINCI_DA850_EVM) += vpif_display.o
> +#DA850 EVM Capture driver
> +obj-$(CONFIG_CAPTURE_DAVINCI_DA850_EVM) += vpif_capture.o

Repeating these lines in the makefile for every board that
gets VPIF support seems certainly excessive. Instead why not
convert the existing DM646x specific config variables to
generic VPIF display and capture config variables and select
these generic variables when someone chooses DA850 support.

Thanks,
Sekhar

RE: [PATCH 1/4] davinci: vpif: add check for genuine interrupts in the isr

[Nori, Sekhar]

Hi Manju,

On Wed, Jan 25, 2012 at 20:35:31, Hadli, Manjunath wrote:
> add a condition to in the isr to check for interrupt ownership and

"to" is misplaced here?

> channel number to make sure we do not service wrong interrupts.
>
> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>

I think it will be nice to expand on the "why" this patch
is required a little bit.

What is the usage case where you can get wrong interrupts?
What exactly happens if you service wrong interrupts?

Explaining these in the commit message will help the maintainers
take a call on the criticality of this patch (whether it should
be queued in the current -rc cycle or not).

Thanks,
Sekhar