[PATCH 2/2] ARM: da850/omap-l138: Add EVM specific code for VPIF to work

[prabhakar.lad@ti.com (Prabhakar Lad)]

Hi Sekhar,

Thanks for the review.

On Tuesday 14 August 2012 06:30 PM, Sekhar Nori wrote:
> Hi Prabhakar,
> 
> On 7/24/2012 1:13 PM, Prabhakar Lad wrote:
>> From: Manjunath Hadli <manjunath.hadli@ti.com>
>>
>> Include the expander settings to select VPIF peripheral on
>> UI card and add registration call in EVM init. Also add platform
>> data to configure display and capture devices.
>>
>> Signed-off-by: Manjunath Hadli <manjunath.hadli@ti.com>
>> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
>> Cc: Sekhar Nori <nsekhar@ti.com>
>> ---
>>  arch/arm/mach-davinci/Kconfig           |    9 ++
>>  arch/arm/mach-davinci/board-da850-evm.c |  195 +++++++++++++++++++++++++++++++
>>  2 files changed, 204 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/Kconfig b/arch/arm/mach-davinci/Kconfig
>> index 32d837d..19ee4ee 100644
>> --- a/arch/arm/mach-davinci/Kconfig
>> +++ b/arch/arm/mach-davinci/Kconfig
>> @@ -190,6 +190,15 @@ config DA850_UI_RMII
>>  	  NOTE: Please take care while choosing this option, MII PHY will
>>  	  not be functional if RMII mode is selected.
>>  
>> +config DA850_UI_SD_VIDEO_PORT
>> +	bool "Video Port Interface"
>> +	help
>> +	  Say Y if you want to use Video Port Interface (VPIF) on the
>> +	  DA850/OMAP-L138 EVM. The Video decoders/encoders are found on the
>> +	  UI daughter card that is supplied with the EVM.
>> +	  NOTE: Please make sure to disable RMII and CLCD options when you
>> +	  select Video Port Interface.
> 
> RMII is in the same choice block as this so it is not possible to have
> RMII enabled along with video port selection. On the CLCD, it will be
> good to register only one of CLCD or video and print a warning if user
> enabled both. davinci_evm_init() in
> arch/arm/mach-davinci/board-dm644x-evm.c has an example of this.
> 
 The CLCD support is not in mainlined yet, if it is the option to enable
the CLCD will also come in the same choice block if  I am not wrong. So
by any means the user can select only one of them.

>> +
>>  endchoice
>>  
>>  config DA850_WL12XX
>> diff --git a/arch/arm/mach-davinci/board-da850-evm.c b/arch/arm/mach-davinci/board-da850-evm.c
>> index 0149fb4..1f5d2c1 100644
>> --- a/arch/arm/mach-davinci/board-da850-evm.c
>> +++ b/arch/arm/mach-davinci/board-da850-evm.c
>> @@ -45,6 +45,8 @@
>>  #include <mach/aemif.h>
>>  #include <mach/spi.h>
>>  
>> +#include <media/tvp514x.h>
>> +
>>  #define DA850_EVM_PHY_ID		"davinci_mdio-0:00"
>>  #define DA850_LCD_PWR_PIN		GPIO_TO_PIN(2, 8)
>>  #define DA850_LCD_BL_PIN		GPIO_TO_PIN(2, 15)
>> @@ -121,6 +123,12 @@ static struct spi_board_info da850evm_spi_info[] = {
>>  	},
>>  };
>>  
>> +#define TVP5147_CH0		"tvp514x-0"
>> +#define TVP5147_CH1		"tvp514x-1"
>> +
>> +#define VPIF_STATUS		0x002c
>> +#define VPIF_STATUS_CLR		0x0030
>> +
>>  #ifdef CONFIG_MTD
>>  static void da850_evm_m25p80_notify_add(struct mtd_info *mtd)
>>  {
>> @@ -452,6 +460,15 @@ static void da850_evm_ui_keys_init(unsigned gpio)
>>  	}
>>  }
>>  
>> +#ifdef CONFIG_DA850_UI_SD_VIDEO_PORT
>> +static inline void da850_evm_setup_video_port(int video_sel)
>> +{
>> +	gpio_set_value_cansleep(video_sel, 0);
>> +}
>> +#else
>> +static inline void da850_evm_setup_video_port(int video_sel) { }
>> +#endif
>> +
>>  static int da850_evm_ui_expander_setup(struct i2c_client *client, unsigned gpio,
>>  						unsigned ngpio, void *c)
>>  {
>> @@ -497,6 +514,8 @@ static int da850_evm_ui_expander_setup(struct i2c_client *client, unsigned gpio,
>>  
>>  	da850_evm_setup_emac_rmii(sel_a);
>>  
>> +	da850_evm_setup_video_port(sel_c);
>> +
>>  	return 0;
>>  
>>  exp_setup_keys_fail:
>> @@ -1149,6 +1168,151 @@ static __init int da850_evm_init_cpufreq(void)
>>  static __init int da850_evm_init_cpufreq(void) { return 0; }
>>  #endif
>>  
>> +/* Retaining these APIs, since the VPIF drivers do not check NULL handlers */
>> +static int da850_set_vpif_clock(int mux_mode, int hd)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int da850_setup_vpif_input_channel_mode(int mux_mode)
>> +{
>> +	return 0;
>> +}
>> +
>> +int da850_vpif_setup_input_path(int ch, const char *name)
>> +{
>> +	return 0;
>> +}
> 
> Why not enhance the VPIF driver to handle NULL values for these instead?
> 
  Ok can be done.
>> +
>> +#if defined(CONFIG_DA850_UI_SD_VIDEO_PORT)
>> +/* VPIF capture configuration */
>> +static struct tvp514x_platform_data tvp5146_pdata = {
>> +	.clk_polarity = 0,
>> +	.hs_polarity = 1,
>> +	.vs_polarity = 1
>> +};
>> +#endif
>> +
>> +#define TVP514X_STD_ALL (V4L2_STD_NTSC | V4L2_STD_PAL)
>> +
>> +static struct vpif_subdev_info da850_vpif_capture_sdev_info[] = {
>> +#if defined(CONFIG_DA850_UI_SD_VIDEO_PORT)
> 
> Why not just continue with the previous #ifdef
> CONFIG_DA850_UI_SD_VIDEO_PORT?
> 
 Ok

>> +	{
>> +		.name	= TVP5147_CH0,
>> +		.board_info = {
>> +			I2C_BOARD_INFO("tvp5146", 0x5d),
>> +			.platform_data = &tvp5146_pdata,
>> +		},
>> +		.input = INPUT_CVBS_VI2B,
>> +		.output = OUTPUT_10BIT_422_EMBEDDED_SYNC,
>> +		.can_route = 1,
>> +		.vpif_if = {
>> +			.if_type = VPIF_IF_BT656,
>> +			.hd_pol = 1,
>> +			.vd_pol = 1,
>> +			.fid_pol = 0,
>> +		},
>> +	},
>> +	{
>> +		.name	= TVP5147_CH1,
>> +		.board_info = {
>> +			I2C_BOARD_INFO("tvp5146", 0x5c),
>> +			.platform_data = &tvp5146_pdata,
>> +		},
>> +		.input = INPUT_SVIDEO_VI2C_VI1C,
>> +		.output = OUTPUT_10BIT_422_EMBEDDED_SYNC,
>> +		.can_route = 1,
>> +		.vpif_if = {
>> +			.if_type = VPIF_IF_BT656,
>> +			.hd_pol = 1,
>> +			.vd_pol = 1,
>> +			.fid_pol = 0,
>> +		},
>> +	},
>> +#endif
>> +};
>> +
>> +static const struct vpif_input da850_ch0_inputs[] = {
>> +	{
>> +		.input = {
>> +			.index = 0,
>> +			.name = "Composite",
>> +			.type = V4L2_INPUT_TYPE_CAMERA,
>> +			.std = TVP514X_STD_ALL,
>> +		},
>> +		.subdev_name = TVP5147_CH0,
>> +	},
>> +};
>> +
>> +static const struct vpif_input da850_ch1_inputs[] = {
>> +	{
>> +		.input = {
>> +			.index = 0,
>> +			.name = "S-Video",
>> +			.type = V4L2_INPUT_TYPE_CAMERA,
>> +			.std = TVP514X_STD_ALL,
>> +		},
>> +		.subdev_name = TVP5147_CH1,
>> +	},
>> +};
>> +
>> +static struct vpif_capture_config da850_vpif_capture_config = {
>> +	.setup_input_channel_mode = da850_setup_vpif_input_channel_mode,
>> +	.setup_input_path = da850_vpif_setup_input_path,
>> +	.subdev_info = da850_vpif_capture_sdev_info,
>> +	.subdev_count = ARRAY_SIZE(da850_vpif_capture_sdev_info),
>> +#if defined(CONFIG_DA850_UI_SD_VIDEO_PORT)
> 
> Same here.
> 
  Ok
>> +	.chan_config[0] = {
>> +		.inputs = da850_ch0_inputs,
>> +		.input_count = ARRAY_SIZE(da850_ch0_inputs),
>> +	},
>> +	.chan_config[1] = {
>> +		.inputs = da850_ch1_inputs,
>> +		.input_count = ARRAY_SIZE(da850_ch1_inputs),
>> +	},
>> +#endif
>> +	.card_name      = "DA850/OMAP-L138 Video Capture",
>> +};
>> +
>> +/* VPIF display configuration */
>> +static struct vpif_subdev_info da850_vpif_subdev[] = {
>> +	{
>> +		.name	= "adv7343",
>> +		.board_info = {
>> +			I2C_BOARD_INFO("adv7343", 0x2a),
>> +		},
>> +	},
>> +};
>> +
>> +static const char const *vpif_output[] = {
>> +	"Composite",
>> +	"Component",
>> +	"S-Video",
>> +};
>> +
>> +static struct vpif_display_config da850_vpif_display_config = {
>> +	.set_clock	= da850_set_vpif_clock,
>> +	.subdevinfo	= da850_vpif_subdev,
>> +	.subdev_count	= ARRAY_SIZE(da850_vpif_subdev),
>> +	.output		= vpif_output,
>> +	.output_count	= ARRAY_SIZE(vpif_output),
>> +	.card_name	= "DA850/OMAP-L138 Video Display",
>> +};
>> +
>> +#if defined(CONFIG_VIDEO_DAVINCI_VPIF_DISPLAY) ||\
>> +		defined(CONFIG_VIDEO_DAVINCI_VPIF_DISPLAY_MODULE)
>> +#define HAS_VPIF_DISPLAY 1
>> +#else
>> +#define HAS_VPIF_DISPLAY 0
>> +#endif
>> +
>> +#if defined(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE) ||\
>> +		defined(CONFIG_VIDEO_DAVINCI_VPIF_CAPTURE_MODULE)
>> +#define HAS_VPIF_CAPTURE 1
>> +#else
>> +#define HAS_VPIF_CAPTURE 0
>> +#endif
>> +
>>  #ifdef CONFIG_DA850_WL12XX
>>  
>>  static void wl12xx_set_power(int index, bool power_on)
>> @@ -1375,6 +1539,37 @@ static __init void da850_evm_init(void)
>>  		pr_warning("da850_evm_init: suspend registration failed: %d\n",
>>  				ret);
>>  
>> +	if (HAS_VPIF_DISPLAY || HAS_VPIF_CAPTURE) {
>> +		ret = da850_register_vpif();
>> +		if (ret)
>> +			pr_warn("da850_evm_init: VPIF setup failed: %d\n",
>> +				   ret);
>> +	}
>> +
>> +	if (HAS_VPIF_CAPTURE) {
>> +		ret = davinci_cfg_reg_list(da850_vpif_capture_pins);
>> +		if (ret)
>> +			pr_warn("da850_evm_init: VPIF capture mux failed:%d\n",
>> +					ret);
>> +
>> +		ret = da850_register_vpif_capture(&da850_vpif_capture_config);
>> +		if (ret)
>> +			pr_warn("da850_evm_init: VPIF capture setup" \
>> +			" failed: %d\n", ret);
> 
> Please don't break print messages.
> 
 Ok.

>> +	}
>> +
>> +	if (HAS_VPIF_DISPLAY) {
>> +		ret = davinci_cfg_reg_list(da850_vpif_display_pins);
>> +		if (ret)
>> +			pr_warn("da850_evm_init : VPIF capture mux failed" \
>> +			": %d\n", ret);
>> +
>> +		ret = da850_register_vpif_display(&da850_vpif_display_config);
>> +		if (ret)
>> +			pr_warn("da850_evm_init: VPIF display setup" \
>> +			" failed: %d\n", ret);
> 
> Same here.
> 
 Ok.

Thx,
--Prabhakar

> Thanks,
> Sekhar
>